review

name	review
description	Code review — self-review before finalization, giving review, receiving review feedback. Use when user says "review", "check the code", "feedback", "review comments", "quality check", or is in a review cycle.
compatibility	macOS/Linux, git, testing tools for verification.
requires	["workspace","platforms","code"]
companions	["requesting-code-review"]
triggers	{"priority":40,"keywords":["\\b(review|check the code|check my code|feedback|quality check|code review)\\b"]}
search_hints	["review","feedback","check the code"]
metadata	{"version":"0.0.1","subagent_safe":false}

Code Review

Delegation

This skill delegates the generic review doctrine to:

• requesting-code-review — when to request an independent review pass
• verification-before-completion — proof before any “review-ready” claim

These are optional companion skills from obra/superpowers. If not installed, this skill still works — you just won't get the external review and verification guidelines. TeaTree keeps the platform-specific review workflow locally: MR discussion handling, inline draft-note rules, and repo policy checks.

Both self-review and external review cycles.

Dependencies

• workspace (required) — provides environment context. Load /t3:workspace now if not already loaded.
• Framework/language convention skills (when reviewing backend code) — e.g., Django conventions, Python style guides. TeaTree auto-detects the relevant ac-* skill from the repo shape. If the loader didn't fire, self-load the appropriate coding skill: /ac-python for Python code, /ac-django for Django projects.

Workflows

Spawn the t3:reviewer Sub-Agent Before Pushing (Non-Negotiable)

Self-review by the implementing conversation never satisfies the shipping gate's reviewing phase. The implementer's context carries every "looks done" blind spot that allowed the gap in the first place — that is exactly what produced souliane/teatree#545's six rounds of follow-up review fixes (missed renames, broken tests, undocumented contract changes, bypassed FSM). The corrective is an independent sub-agent that hasn't seen the implementation conversation.

The only sanctioned path to advancing a ticket from TESTED → REVIEWED is:

1. Spawn the t3:reviewer sub-agent from the main conversation via the Agent tool. The full Agent invocation snippet, FSM transition mechanics, and "drive transitions, not visit phases" rules live in ../ship/SKILL.md § "Review Gate" — that section is the source of truth.
2. Apply every finding the sub-agent surfaces. Reviewer agents are read-only; the implementing conversation owns the edits.
3. Drive the FSM review transition (by completing the reviewing task, which auto-fires ticket.review()). Never use t3 <overlay> lifecycle visit-phase reviewing as a substitute — it writes the session record without advancing the FSM, leaving Ticket.state stuck at TESTED.

The "Self-Review Before Finalization" workflow below is a complement to the sub-agent pass, not a replacement. Run it first to catch the obvious things, then spawn the reviewer.

Self-Review Before Finalization

Review ALL diverging code, not just the last commit:

git diff --merge-base main

Precondition — branch must be current with main. If main has advanced since the branch's merge-base, the diff will surface those new commits as phantom "reversions" — code the author looks like they deleted but actually never had. Reviewing on top of a stale branch produces spurious scope-creep findings AND can let real silent-revert PRs through.

git fetch origin main --quiet
git merge-base --is-ancestor origin/main HEAD || git merge origin/main --no-edit

Run this before the cleanup checklist. Resolve any conflicts the same way you would on a normal merge — no rebase, no stash.

Cleanup checklist:

• No code duplication introduced
• No dead code left behind
• Routing reachability: every modified component is reachable via the target flow's route tree. Read the relevant routes.ts and confirm the component (or its parent shell) appears there. If the component lives in a flow-specific folder (e.g., natural-person-calculation/), verify the target flow actually routes through it.
• Naming follows project conventions
• Patterns match existing codebase
• No debug/temporary code remaining

Active Verification Against Repo Rules (Non-Negotiable)

After the cleanup checklist, actively verify each changed file against the repo's agent config files (AGENTS.md or the repo's equivalent agent instructions file) — not as a passive reminder, but as a file-by-file gate:

1. Read the repo's agent config files (e.g., AGENTS.md or the repo's equivalent agent instructions file).
2. For each changed file, check against every applicable rule section. Focus on:

• Architectural patterns (e.g., container-presentational, signals-first, inject vs constructor)
• Feature flag and multi-tenant rules (see references/multi-tenant-development.md § Review Checklist)
• Banned patterns (e.g., manual .subscribe(), any types, hardcoded strings)

3. Check consistency across the changeset — if the same pattern is applied differently in two files within the same MR, that's a finding.
4. When a repo rule conflicts with a teatree or overlay skill rule, do NOT silently pick one. Present both rules to the user with the specific conflict, ask which takes precedence, and save their decision to the agent's memory for future reference.

This step catches the class of bugs where the rules exist but weren't applied during implementation — missed feature flags, wrong DI pattern, manual subscriptions where signals were required, etc.

Module-Level Architectural Check (Non-Negotiable)

After verifying repo rules, check the full file (not just changed lines) of every file touched by the diff against the loaded coding skills' "Architectural Health" review checklist.

1. Identify loaded coding skills. TeaTree auto-detects ac-* skills from the repo shape (e.g., ac-python, ac-django). If they have an "Architectural Health" review checklist section, apply it.
2. For each touched file, evaluate the FULL file against those checklists. Key checks (skill-specific details are in the skill itself):
   • Module size (LOC)
   • Module-level function count and justification
   • God-module detection (unrelated concerns in one file)
   • Complexity rule suppressions in pyproject.toml — any C901/PLR09xx per-file-ignores beyond the project's boilerplate baseline are findings
3. When a threshold is crossed, either refactor to comply or create a ticket for the debt — do not suppress the lint rule.
4. Check pyproject.toml per-file-ignores for the touched files. If any suppress complexity rules that are not in the project's boilerplate baseline, flag them as findings.

This step prevents architectural drift. Each diff looks fine in isolation — this check catches the cumulative effect by examining the full module.

New-Test Shape Check (Non-Negotiable)

When the diff adds or modifies test files, verify the new tests follow the repo's test-writing doctrine (see the repo's AGENTS.md § "Test-Writing Doctrine" — teatree and every overlay repo carry the same rule):

1. Mock density. If a new test file is mostly Mock(), patch(), MagicMock, or mock.call_args assertions, flag it. Ask: could this have been a Django test client call, a call_command invocation, a real tmp_path git repo, or a Playwright E2E?
2. Mock targets. Mocks should hit unstoppable externals only — network (GitHub, GitLab, Slack, Sentry), clock, pass, third-party subprocesses. Mocking teatree code, Django models, filesystem under tmp_path, or git itself is a finding.
3. Missing integration coverage. If the diff adds a view, a management command, or a new CLI surface and only ships unit tests, flag it — the happy path belongs in an integration test.
4. Coverage preservation. Any test rebalancing (removing units, adding integration) must keep the coverage gate satisfied. Report the before/after coverage number in the review.

Accept a mock-heavy test only when the MR description justifies why a higher-level test couldn't cover the same behavior (e.g., a rare error branch that's painful to trigger through the real entry point).

Quality Gate Verification (Verify-Fix-Repeat)

Before declaring review-ready, run all gates and iterate until they pass. Do not declare review-ready after a single pass — re-run gates after every fix, because fixes can introduce new failures.

Run gates → Any failure? → Fix → Re-run gates → Repeat until clean

Gates (run in order):

1. Lint: zero errors from the project linter
2. Type check: passes (if the project uses it)
3. Tests: full suite green (use t3 <overlay> run tests or project equivalent)
4. No uncommitted changes: all fixes staged and committed
5. No regressions: diff review confirms no unintended changes

Iteration limit: After 3 fix-verify cycles without convergence, stop and ask the user — the issue may be systemic rather than incremental.

Stop hook integration: If the repo has a Stop hook (in the agent's settings), it enforces this loop automatically. Without a hook, run the gates manually before claiming done.

References: Ralph Loop (external verification over self-assessed completion), Effective harnesses for long-running agents (Anthropic, feature-list-driven incremental verification).

Giving Code Review

Pre-flight gate — complete BEFORE reading any diff:

1. Determine own vs external MR (Step -1)
2. Fetch ticket context for every MR (Step 0) — without this you cannot judge correctness
3. List all commits per MR (Step 0b)
4. Read the repo's AGENTS.md / agent instructions file and any project-specific coding guidelines

Do NOT skip these steps to "save time" when reviewing multiple MRs. Each step exists because skipping it caused missed findings in real reviews.

Step -1 — Own MR vs External MR:

When the MR under review belongs to the user themselves, do NOT post review comments. Instead, implement the fixes directly on the branch — commit and push. Present findings to the user as a summary of what you fixed, not as review comments to post. The user is asking you to take over and improve their code, not to leave notes for themselves.

Step 0 — Gather Ticket Context:

Before reading any code, fetch the referenced ticket/issue to understand the intended behavior:

1. Extract the ticket URL or number from the MR title/description.
2. Fetch the issue via the project's issue tracker CLI (e.g., glab issue view, gh issue view).
3. Fetch every attached spec (PDFs, OpenAPI files, vendor docs) and every linked external requirement. For GitLab attachments, the working path is glab api projects/<id>/uploads/<secret>/<filename> — browser-style URLs (gitlab.com/<group>/<repo>/uploads/..., gitlab.com/-/project/<id>/uploads/...) require session cookies and return login HTML when hit with a PAT. Attachments are the authoritative spec; an author docstring summarising them is not a substitute.
4. If external requirements links are referenced, fetch those too.
5. Use the ticket context + attachments as the ground truth for evaluating correctness.

Hard rule — refuse blind reviews. If a ticket references a spec attachment or external requirements document that you cannot retrieve, STOP. Do not post review notes. Report back to the user: which document you couldn't fetch, what you tried, and what permission / access / exception is needed. Overlay skills MAY declare specific sources as out-of-scope (partner portals behind SSO the sandbox cannot reach, for example); honour those per-overlay exceptions. For anything else, a review with missing spec context is not a review — it's guessing, and guessing attached to the user's account damages the author's trust.

Without ticket context you cannot judge whether the implementation is correct — only whether it compiles.

Step 0b — Review All Commits, Not Just the Final Diff:

The combined diff can hide mistakes. Always check individual commits:

1. List all MR commits (e.g., glab api .../merge_requests/<IID>/commits).
2. Inspect each commit's diff individually — a later commit may accidentally revert an earlier fix.
3. Look for "Tests fix" / "Fix tests" follow-up commits that change production code alongside test adjustments.

Step 0c — Discuss Before Posting:

Present ALL findings to the user before posting any comments. Never silently drop findings between the discussion phase and the posting phase — if a finding was discussed, it gets posted unless the user explicitly removes it. The user curates; you surface.

When raising concerns about caching, stale data, or side effects: investigate first. Check the actual code paths and real data before speculating. A concern backed by evidence ("I checked the DB — durations do vary") is useful; a speculative "this might be a problem" wastes the author's time.

Step 0d — Answer Your Own Questions Before Posting (Non-Negotiable):

Every review comment is posted under the user's name. A comment that boils down to "I'm unsure, please confirm" makes the user look like they don't know their own codebase. Do not post it.

Before drafting any comment, if it would contain any of the following phrases — or their equivalents — STOP and investigate first:

• "worth confirming with the business that…"
• "worth checking <file> / <function> / the downstream serializer / etc."
• "can you confirm this value matches what upstream emits?"
• "is this string / identifier / enum value correct?"
• "does this field exist in the producer schema?"
• "I'm not sure whether…"
• "does this mean that… / or… / or…?" (listing options instead of picking one)
• "verify that …" / "please check …" / "confirm whether …" — any imperative that asks the author to do verification work the reviewer is capable of doing themselves.

The reviewer does the verification, not the author. If the comment names a file, function, schema, enum, downstream caller, or any other artifact reachable from the local checkout, open it and read it before posting. "Worth checking foo.py" is not a review comment — it is the reviewer outsourcing their job. Either the file says the code is wrong (post a verified finding) or it says the code is fine (post nothing).

Investigate first by exhausting the sources you can reach:

1. Grep the repo for the symbol / string / identifier — producers, consumers, enums, tests, fixtures, docs.
2. Grep sibling repos when the value crosses a service boundary (e.g., webhook producer → consumer, API schema → client). The upstream producer's source of truth lives there. Discover sibling repos via T3_WORKSPACE_DIR or the overlay's configured repo list — never hardcode a user-specific path.
3. Read the producer's schema / enum / migration — whichever repo emits the value. If it's a Django model, check the field's choices= and the migration history. If it's a Pydantic model, check the field type.
4. Check commit history for the rename, addition, or removal — git log -S "<symbol>" --all --oneline often shows exactly when and why the value changed.
5. Read the test fixtures — realistic test inputs show what the producer actually sends.
6. Check related MRs on the same or upstream repos for the same symbol — someone may have already merged or discussed it.

Only after all reachable sources are exhausted can you post a question-style comment — and only when the answer truly requires access you do not have (partner portal behind SSO, vendor-only documentation, product owner's desk knowledge). State what you checked and why the answer isn't reachable, so the author sees you did the work.

Scale severity to confidence. A speculative "maybe wrong?" is a nit at best; drop it. A verified finding ("grepped foo-producer, canonical spelling is X, branch has Y — will fail at runtime") is a blocker and belongs in the review.

When the investigation confirms the code is correct, say nothing. Silence on a check you performed is the correct outcome — not a "looks good, but…" comment. Positive comments belong in the summary to the user, not in the MR.

Step 0e — Don't Police Other Authors' Title/Description Format (Non-Negotiable):

Do NOT leave review comments about an external author's MR title format, description wording, commit-message style, work-item link spacing, or whether their description "reads better" in a different shape. These rules are enforced by CI and by the overlay's validate_pr() check — not by the reviewer. Raising them manually duplicates the bot and nags a colleague for something a machine already polices.

The reviewer's responsibility is to ensure their own MRs pass the title/description check. On other authors' MRs, silence on formatting is the correct outcome. If something is objectively wrong in a way that affects traceability or release notes (e.g., the title references the wrong ticket), frame it as a correctness finding, not as a style nit.

Step 0f — Respect the Overlay's Auto-Close Policy (Non-Negotiable):

Do NOT suggest adding Closes #NNN, Fixes #NNN, Resolves #NNN, or any other auto-close keyword to an MR description unless the active overlay's conventions explicitly require it. Many overlays manage issue closure via their own ticket/MR linking rather than via GitHub-style auto-close trailers, and suggesting them contradicts the overlay convention.

Check the overlay skill's commit-message and MR-description rules before proposing any trailer. The default when the overlay is silent on the topic is: do not suggest auto-close trailers.

Step 0g — Cross-Service Verification (Non-Negotiable):

A review of a service that talks to other services is incomplete until those other services have been checked. Reviewing one repo in isolation produces blind comments — the reviewer asserts "this is the convention" or "this default is fine" without knowing what the producers and consumers across the platform actually do. Comments built on that premise make the user look stupid when the author replies with "have you checked the FE / the gateway / the sibling microservice?".

Before posting any comment about a name, contract, default value, schema field, response shape, or wire format, exhaust the cross-service grep:

1. Enumerate the related services at the start of the review. From the MR's repo, list every service that produces or consumes the same data: upstream gateway, downstream consumers (frontend, sibling backend, document generation, data warehouse), shared schema/proto repos. Discover them via T3_WORKSPACE_DIR (or the overlay's configured repo list) — never hardcode a user-specific path. State the list explicitly so the user can correct gaps before you start.
2. Grep every related service for the symbol/string/identifier/field name. Frontend models, backend serializers, fixture files, generated docs, OpenAPI specs, migration histories. Note where each occurrence lives.
3. Cite the cross-service evidence in the comment. "Frontend has 18 references to idExpirationDate in libs/shared/data-model/...; the gateway-side Python repo has matching references in docgen/serializers/..." is a finding. "I think this should be spelled differently" is a guess.
4. When the cross-service check reveals the comment was wrong, drop the comment. A comment that survives the check survives because the platform-wide convention contradicts the diff. Silence is the correct outcome on a check that confirmed the diff is fine.
5. When the cross-service check is impossible (a repo is not in scope, sandboxed, or behind access you don't have), say so explicitly in the comment and name what was checked vs not. Don't pretend you ran a check you didn't.

Triggers for this step — every diff touching:

• A schema field name, enum value, or wire format (Pydantic models, DRF serializers, TypeScript interfaces, OpenAPI definitions).
• A default value or boolean flag that previously had a different default (especially flags with always-on/always-off semantics like loyalty enrichment, feature gates, search filters).
• A response shape or wrapper type returned by an endpoint already consumed by another service.
• A query parameter name or required/optional toggle on a public endpoint.
• A renamed function, method, or class that is used cross-repo (gateway client, shared library, public CLI command).

If none of those triggers apply (purely internal refactor, test-only change, comment fix), this step is satisfied automatically.

Failure mode this step prevents: a reviewer posts "the canonical name should be X" based on the local repo's pattern, the author replies "the FE has 20 usages of Y, please check before commenting", and the user (whose name is on the comment) loses credibility for a finding that would have been correct if the reviewer had grepped the FE first.

Step 0h — Discussion Venue: MR Over Chat (Non-Negotiable):

Discussion topics that anchor to specific code in an MR — design questions about a function, a TODO in the diff, a missing call compared to a sibling endpoint, a hardcoded value, an architectural choice visible in the patch — belong on the MR, not in a Slack/Teams DM or chat thread. Default to MR notes whenever the topic references something the reviewer can point to in the diff.

Why MR over chat:

• MR notes are persistent, threaded per topic, and resolve with the MR. Chat scrolls away.
• Other reviewers and stakeholders see MR notes; chat is a private channel between two people.
• MR notes attach to the line/file, so the conversation stays anchored to the code that triggered it.
• The ticket's audit trail benefits from the discussion living next to the code change.

When chat is the right venue:

• A heads-up that the review is ready and points to the MR for the discussion ("left some thoughts on !351").
• Coordination/scheduling ("can we pair on the LE flow tomorrow?").
• Sensitive feedback that doesn't belong in a public review trail.
• Topics genuinely unrelated to the diff (e.g., process discussion about how the team reviews MRs).

Inline first, general note second:

When posting on the MR, prefer inline (line-anchored) discussions over general MR comments. Inline notes show the exact code that triggered the question and let the author resolve them per topic. Use a general MR comment only when the topic is not anchorable to a single line — for example, an architectural question that spans the whole file or a code block that is not part of the MR's diff (so GitLab cannot anchor an inline note to it).

Failure mode this step prevents: the reviewer drafts a Slack message containing 5 design questions about specific lines of an MR, sends it as a DM, and the discussion lives in chat where it is invisible to the rest of the team and disconnected from the code. The author then has to copy-paste the chat back into MR comments to track resolution. The right move was to post the topics as MR discussions in the first place and send a one-line Slack heads-up pointing to the MR.

Step 1 — Structured Review Checklist:

1. Correctness — does the code do what the ticket requires? Are all acceptance criteria met? When a change tightens a public contract (e.g., serializer field becomes required, API parameter becomes mandatory), trace all callers — the change affects every flow that uses that interface, not just the one the ticket describes.
2. Completeness — are there missing production code changes that the tests assume? Do test expectation changes have matching implementation changes?
3. Feature flag — follow the review checklist in references/multi-tenant-development.md. Before raising a "missing feature flag" finding, trace the full gating chain upward — the component under review may not have a flag itself but could be hidden/disabled at the container or routing level (e.g., hidden: !featureFlagService.hasFeatureFlag(...) in the parent that renders it). A finding is only valid if the feature is reachable without the flag.
4. Style — follows project conventions?
5. Tests — adequate coverage of new behavior?
6. Safety — no security issues, no data loss risks?
7. Migrations — reversible? data-safe? performance-safe?
8. Scope — are unrelated changes bundled in? Flag only if genuinely unrelated; small related fixes alongside the main change are normal practice.
9. MR metadata — title and description comply with the overlay's commit message format? If the overlay provides validate_pr(), run it programmatically rather than checking by eye.

Step 2 — Review Tone & Formatting:

Follow the Google Engineering Practices — Code Review Standard: approve if the CL improves overall code health, even if it isn't perfect. Don't block on style preferences or theoretical improvements. The bar is "does this improve the codebase?" — not "is this how I would have written it?"

Comments are posted under the user's name. They must sound like a real human colleague wrote them — not an AI, not a linter, not a manager.

Voice & attitude:

• Be the best colleague. Helpful, curious, humble. Happy to teach, never to humiliate. You're a peer who genuinely wants the code (and the author) to succeed.
• Never parent. Don't lecture, don't explain things the author obviously knows. If you're providing context, frame it as "in case it helps" or "I think this might…" — not "you should be aware that…".
• Be collegial. Phrase observations as questions or suggestions, not orders. "Would it make sense to…?" beats "You must…".
• Assume good intent. A reverted line is more likely an accidental rebase artifact than carelessness. Frame it that way.
• Acknowledge what's good. If the approach is sound, say so briefly before raising issues.
• Scale severity to impact. A missing production code change that breaks tests is critical. A minor style nit is not. Don't escalate small things.
• Never demand separate tickets/MRs for minor scope additions. A small related fix alongside the main change is normal — only raise scope if genuinely orthogonal work is smuggled in.

Formatting rules:

• Prefix nits. When a comment is nitpicking (style, naming, minor preference), prefix with Nit: so the author knows it's non-blocking.
• Backticks for code. Always wrap code symbols, class names, method names, variable names, file paths, and CLI commands in backticks (`).
• Use suggestion blocks for concrete code changes. When you have a specific replacement in mind, use the platform's suggestion feature (```suggestion fenced block on both GitLab and GitHub) so the author can accept with one click. GitLab supports :-N+M to expand the range. Combine explanation text before the suggestion block.
• Readable structure for longer comments. Use empty lines to separate distinct sections (problem, suggestion, example). Within a section, use line breaks between sentences (without empty lines) to keep things scannable. Short comments stay on one line — don't over-structure a one-liner.
• No walls of text. If a comment needs more than ~5 lines, break it up visually. Paragraphs, not monoliths.

Step 3 — Post Draft Review Comments:

Always use draft notes (or the platform's equivalent "pending review" feature), not direct/immediate comments. Draft notes are only visible to the reviewer until explicitly submitted — this lets the user review, edit, and submit all comments as a batch.

Pre-flight: read existing comments (Non-Negotiable). Before posting any new comments, fetch all existing discussions and notes on the MR (from all authors, not just the current user):

1. List all discussions via GET .../merge_requests/<IID>/discussions?per_page=100 and read each note's body.
2. For each finding, check whether an existing comment already raises the same concern — same file, same line range, same substance. If so, do not post a duplicate.
3. If you have something to add to an existing discussion (additional context, a related concern on the same code), reply in that thread via t3 review reply-to-discussion <REPO> <MR_IID> <DISCUSSION_ID> "body" instead of creating a new top-level comment.
4. Only post new draft notes for findings not already covered by existing comments.

This prevents noise from multiple review passes or multiple reviewers covering the same ground.

Post all new findings. Don't self-censor or hold back comments because they seem minor. The user will review every draft note in the platform's UI, edit wording, and delete anything they don't want before submitting. Your job is to surface everything you noticed — the user curates. But "everything" means everything not already said — duplicating an existing comment wastes the author's time.

When reviewing an external MR/PR, always post comments inline on the correct file and line in the diff view. For comments that aren't tied to a specific line (e.g., description feedback), post a general note without position data.

Extend the CLI, never inline API recipes. If a t3 review operation is missing, implement it in src/teatree/cli/review.py — do NOT document a raw API snippet or inline script here. Skills describe what command to run, not how to replicate missing CLI functionality. Current subcommands: post-draft-note, delete-draft-note, publish-draft-notes, list-draft-notes, update-note, reply-to-discussion, resolve-discussion.

Use t3 review post-draft-note (Mandatory). It handles token extraction, diff refs, position serialization, and added-line validation. Never use raw API calls.

t3 review post-draft-note <REPO> <MR_IID> "Comment text" --file <path/to/file> --line <line_number>

Pre-flight: verify target line is an added line. Before posting each inline note, confirm the target new_line corresponds to an added (+) or modified line in the diff — never a context (unchanged) line. Targeting a context line causes GitLab to render the comment in every hunk that references that line, creating duplicates. When the finding is about an unchanged line, target the nearest added line and reference the unchanged line in the comment text instead.

Pre-flight: the file you anchor on MUST be the file the body discusses. If the comment body describes code in foo.py (e.g., "foo.py's bar() is missing X that the sibling baz.py got"), anchor the comment on foo.py — not on baz.py, even if baz.py has more added lines in the diff. Two defensible patterns when foo.py has no added lines:

1. Pick the nearest added line in foo.py (even a whitespace or adjacent-line change) and open the body with "Note on an unchanged line below:" so the reader sees the anchor is a stand-in.
2. Post a general (MR-level) note instead of anchoring on a sibling. A comment anchored on the wrong file is worse than a general note — the author opens baz.py looking for the problem, finds nothing, and loses trust in the review.

Post-flight: verify response. Response must confirm the comment landed on the correct file/line — if position data is missing in the response, the comment landed as a general comment (wrong). After posting all notes, list them via the API and confirm the count and positions match expectations.

Do NOT submit the review without explicit user instruction. By default, the user reviews draft notes in the platform's UI, edits if needed, and submits manually. If the user explicitly asks to publish (e.g., "post with t3 cli", "submit the review"), use:

t3 review publish-draft-notes <REPO> <MR_IID>

WARNING: inline position was not accepted means GitLab did not store the position data — the note will render as a general comment, not inline on the diff. Check that --file matches a path in the MR diff and --line is within the changed range.

If t3 review delete-draft-note returns 404 — the draft was already submitted (published to regular notes) by the user from the GitLab UI. Use DELETE projects/{encoded_repo}/merge_requests/{iid}/notes/{note_id} via the regular notes endpoint instead.

Position field reference

Field	GitLab	GitHub
File path	old_path / new_path	path
New line (added/modified)	new_line	line + side=RIGHT
Old line (deleted)	old_line	line + side=LEFT

Receiving Code Review

• User feedback = trusted direction. Verify scope, then implement.
• External reviewer = verify technically before implementing.
• Push back when: suggestion breaks functionality (show evidence), violates YAGNI, is based on stale context, or conflicts with user's stated architecture.
• Anti-performative: No "You're absolutely right!" — just state the fix or the technical disagreement.
• Technical rigor: verify reviewer suggestions against the actual codebase before implementing.

Replying to Review Discussions

When posting replies to reviewer discussions (e.g., "Done in <commit>"):

1. Fetch all discussions via API and inspect each one's first note — read the actual body, don't rely on assumptions about which discussion covers which topic.
2. Match reply to the specific concern. Read each discussion's first note body in full. The reply must use the same framing as the reviewer — if they asked about FeatureFlagService, don't reply about takeUntilDestroyed. Never post a generic "addressed in commit X" reply to a discussion about a different topic.
3. Skip already-answered discussions. If the user (or someone else) already replied with a resolution, do not post a duplicate reply.
4. Present the mapping to the user before posting. Show a table: | Discussion | Topic | Reply | and get confirmation. Never batch-post replies without review.

Post each reply via t3 review reply-to-discussion <REPO> <MR_IID> <DISCUSSION_ID> "body". To mark a thread resolved after the reply, use t3 review resolve-discussion <REPO> <MR_IID> <DISCUSSION_ID> (pass --no-resolved to re-open).

Commands

Command	When to use
t3 ci quality-check	Quality analysis for self-review
t3 <overlay> run tests	Verification after review changes