Skip to content

Decision: Ticket Bar for Review Findings, Enforced at Merge

Date: 2026-10-04 Snapshot commit: branch develop

Background

A PR's code review or live QA turns up findings. Each one can go back to the Dev in the same branch, or become a new ticket. A new ticket throws away context the Dev already has loaded, and a later session has to rebuild it from scratch. A default of filing tickets also keeps the open-issue count from ever falling.

qa.md tightened this rule three times (2026-09-11, 09-13, 09-14; history below). It didn't hold. On 2026-10-04, open issues were back at 111. Over the prior month 75 were opened and 77 closed, and about half of the new ones were spun off PR reviews or retros. Of the 23 spin-off tickets filed after the 09-14 tightening, only 2 named a bar (#757, #878). These counts come from a keyword search of issue bodies, so they're approximate. The clearest case: PR #884's round-1 comment had a "Not blocking (filed as #887)" heading, with no bar named, for small fixes inside the function the PR itself added.

Why the earlier tightenings didn't stick

  1. Wrong place. The rule lived in qa.md's Retro section, a log of lessons learned. The numbered steps QA follows when it writes a verdict (3, 7, 8) never mentioned it, so nothing raised the question at the moment of filing.
  2. More words, same place. Each tightening lengthened the same bullet, which grew to roughly 1,500 words. None changed what QA sees at the decision point.
  3. Self-graded. The agent deciding a finding was "not blocking" was the only one who checked.
  4. A second unguarded path. retro.md's Ticket category had no bar at all (#883 came in that way).
  5. Never measured. No tightening was checked against real filings afterward.

Decisions

  1. The rule moves into the procedure. qa.md step 3 requires a finding list in the PR comment. Every finding is send-back, ticket — bar N: <reason> (#issue), or dropped: <why it isn't a real defect>. No "non-blocking" category exists. A drop is only for a false positive, a non-reproducible finding, or intended behavior; otherwise it would become the next escape hatch once filing costs a named bar (Bob's round-1 review of PR #889). A send-back routes the round to step 7.
  2. Kai enforces it at merge. merge-pr.md step 2 reads the QA comments. A filed ticket with no bar N: reason, or a drop justified by size or priority, blocks the merge and goes back to QA. The ticket query doesn't filter on from-review: a ticket that skipped the bar skips the label too (#887 has neither). Kai is a second agent with no stake in the PR. A bar that's named but plainly doesn't fit goes to the user rather than being decided by Kai alone.
  3. Bar 2 is restated as "outside this PR's diff." The rationale is loaded context. A finding in a function the diff doesn't touch is a fair ticket, because the Dev has no context for it either (#876 is that case). The copy of a pattern inside the diff is still a send-back. Narrowed by decision 9: the boundary is now the file or directly called function, not the function.
  4. Retro tickets use the same bars. retro.md's Ticket category points to them. One exception: once the PR has merged there's no send-back left, so an observed failure is filed as defect with from-review and "merged" as its reason; anything else that clears no bar is recorded in the retro's report, not filed (Bob's round-1 review of PR #889, user decision 2026-10-04).
  5. Measured. Every ticket filed under a bar gets the from-review label. Check the rate two weeks out (around 2026-10-18): gh issue list --label from-review --state all --search "created:>=2026-10-04", compared with the number of merged lane PRs. That checkpoint also covers decisions 9, 10, and 11 (counts by impact × reach cell): count how many from-review tickets were filed for findings in a file the PR modified or a function its code called, and how many went into the in-flight release's label.
  6. Bar 2 gets a value test (2026-10-04, after PRs #903 and #905). Being outside the diff explains why a finding can't be fixed now. It doesn't show the finding is worth tracking. A bar-2 ticket needs behavior impact: an observed failure, or a concrete path to wrong output, a crash, a hang, a leak, or misleading output. A refactor or dedup finding with no behavior impact is dropped, and "outside this diff, no behavior impact" is a valid drop reason.
  7. Search covers planned fixes, not only matching defects. An open issue whose design would also fix the finding absorbs it as a comment, even when its title describes a different symptom.

  8. Dev-filed follow-ups use the same bars. dev.md step 6 applies qa.md step 3's triage to findings a Dev makes while implementing. The Dev files any ticket after opening the PR, cites the PR number in it, and lists it in the PR body as a ticket — bar N: line; unfiled findings go in as dropped: lines. Filing after the PR matters because Kai's merge-check search only finds tickets created after the PR that cite its number (Bob's round-1 review of PR #910). Kai's merge check reads the PR body as well as QA's comments.

  9. Bar 2 is measured per file or called function, not per function (2026-10-12, #995). "Inside the diff" now means every file the diff modifies plus every function the diff's new or changed code calls directly. A finding there with behavior impact is a send-back unless bar 1, 3, or 4 applies. Bar 4 uses the same boundary. Decision 3's loaded-context argument holds less well than it looked: a Dev changing a file has its neighbors loaded, and a helper on the PR's new path is where the PR's own QA sees the failure. A finding in an untouched function with no behavior impact is dropped, so the wider boundary doesn't pull refactors into the PR. No size limit was added (#995 proposed "a few lines plus a unit test"): size is the kind of description decision 1 already rejects as a reason, and bar 4 covers the fixes that actually grow a PR.

  10. Review tickets aren't labeled for the in-flight release by default (2026-10-12, #995). kai.md's triage section gives a from-review ticket the in-flight release's label only if a commit after the last stable tag introduced the defect, or a failure was actually observed (wrong output, a crash, a data exposure). Everything else goes to the next version. Superseded by decision 11's rubric.
  11. Version labels by impact × reach, measured by cell (2026-10-13, #1024). Decision 10's two conditions are folded into kai.md's impact × reach rubric, which applies to every path into a version, not only from-review (decision-impact-reach-triage.md). Decision 5's 2026-10-18 checkpoint also classifies each from-review ticket created since 2026-10-04 by its cell (severe/wrong/loud/hygiene × common/narrow/rare) and reports the count per cell, not only the overall rate. Tickets filed before the rubric don't name a cell, so classify those by hand from the issue text, the way #1024's table was built.

History (moved from qa.md's Retro section, 2026-10-04)

  • Raise the bar on what counts as non-blocking — if a finding is worth writing up, it's worth sending back to the Dev to fix now, in the same pass, using the context already loaded — not filing a ticket that discards it. A code-review or live-QA finding defaults to blocking: comment on the PR and send it back to the Dev in the same branch, the same pass — input-validation gaps, duplicated helpers, a style inconsistency the new code merely repeats from an existing convention, a docstring the PR's own diff made inaccurate. The old default (label these "minor/systemic," approve anyway, file a ticket) was generating far more tickets than the team could close — a single PR's review routinely spun off 1-2 backlog issues nobody would revisit, all describing something that was, by definition, already fully understood and small enough to fix in the moment; filing a ticket for it just discards that context so someone has to re-derive it later. Every new ticket cycle is new context a Dev has to reconstruct from scratch (re-open the file, re-derive the finding, re-load the surrounding code) — a cost paid in tokens and turnaround that a same-round send-back skips entirely, since the Dev already has the file and the finding loaded right now. Optimize for that: fixing it now, in the branch already open, is strictly cheaper than fixing it later in a new one.

A ticket is the exception, and the exception must be named, not implied. Only file a ticket when the finding clears one of exactly four bars — and the PR comment/ticket body must say which one, explicitly, not just assert "non-blocking" or "low priority" and stop there: 1. It needs a maintainer/product decision — a real design or scope call the Dev is structurally blocked from making (changes user-facing behavior, trades off cost/risk only the maintainer can accept, or sets precedent for how the codebase should work beyond this PR). Multiple reasonable ways to implement a fix existing is not evidence of this bar by itself — a Dev picking one of several correct engineering options (which test to add, how much to refactor, whether a comment suffices) is ordinary PR work, not a maintainer decision (see Retro, 2026-09-14). 2. It's a genuinely cross-cutting change spanning far more of the codebase than this PR touches — not just "the same pattern also appears elsewhere," but a multi-call-site sweep the PR's own diff didn't create the need for (e.g. #737: 6+ call sites the PR never touched). 3. It needs infrastructure/access this pass doesn't have. 4. It's a defect already on develop that this PR's ticket doesn't cover, and fixing it would move code to a different thread, or change what awaits what, inside a function this PR's diff already modifies. However short the diff, a fix like that opens new races for the next round to find (see Retro, PR #867). A pre-existing defect whose fix doesn't change threading or await order is not bar 4: send it back.

"Narrow," "contrived repro," "low priority," or "not blocking qa-approved" are descriptions, not exceptions — none of them alone justifies a ticket. If a finding doesn't obviously match one of the four bars above, the default action is to send it back to the Dev in this round, not to file a ticket "to be safe." When a ticket is still warranted, classify it enhancement/backlog (or defect only if a failure was actually observed), same as before. And if a finding genuinely isn't worth fixing at all — not just small, actually not worth anyone's time — don't file a ticket for that either; note it in the PR comment for the record and let it go, rather than manufacturing backlog. Don't hold a PR hostage over a pre-existing convention the PR didn't introduce and can't reasonably fix in isolation — that's the one case that still ships as-is, filed only if it clears the bar above.

Search before filing, every time. Once a finding clears a bar, run gh issue list --state open --search "<function or file name> <symptom>" immediately before gh issue create. Earlier rounds of the same pass, or another lane, may already have ticketed it. If an open issue already describes the same defect (same code path and same failure, not just a shared keyword), comment the new evidence on that issue instead of filing. This applies to every ticket QA files, not only fixture pollution. - Minor/systemic code-review findings judged non-blocking still need to actually get filed — PR #406 round 1's /code-review surfaced two legitimate but non-blocking findings (a readability nit in _emit_block_node's early-return condition, and a _block_had_unsupported_content reset gap on one block-open path) that correctly didn't hold up qa-approved per this file's own "minor/systemic findings are tickets, not blockers" entry above — but they sat unfiled through two more QA rounds until a /retro pass caught the gap and filed them as #417. Command decision: when a round's code-review or live-QA turns up a finding judged non-blocking, file its ticket in the same pass that makes the non-blocking call, not deferred to "later" — "not blocking qa-approved" and "filed" are two separate actions, and only the first one has a step number in this file forcing it to happen. - Don't file a GitHub issue for a finding that's blocking and going back to the Dev in the same review cycle — only non-blocking/deferred findings get a ticket. PR #455 (issue #158): filed #457 for the one blocking finding (missing calendar_cache.mark_dirty calls on the new ACL tools), then had to close it as redundant once realizing this file's own blocking-finding process (comment on the PR, stop, hand back to the Dev) has no ticket step — the PR comment plus the Dev's own fix commit is already the durable record, and a ticket for something about to be fixed in the same PR cycle just gets closed the moment the fix lands. Command decision: before running gh issue create for a code-review or live-QA finding, classify blocking vs. non-blocking first (per this file's own guidance above) — only non-blocking findings get a ticket; a blocking one gets a PR comment and nothing else. - The "minor/systemic findings are tickets" default was itself the problem, not individual mis-classifications. 2026-09-11 (Kai orchestrator session): user flagged that open issues were growing roughly 2:1 against closures (37 opened vs. 20 closed over the prior 14 days, 60 of 128 open issues sitting in unscheduled backlog), and traced it to this file's own prior guidance — every dev-lane PR's /code-review pass was correctly following the old rule (classify as non-blocking, approve, file a ticket) and correctly producing a steady drip of single-PR-follow-up tickets nobody was coming back for (#718, #724, #726, #727 — four in a row, one per recent merge). User's framing: "if it's worth filing, it's worth using the existing context to fix" — the fix was never QA doing unrelated Dev-shaped work (that boundary, above, is unchanged), it's that "non-blocking" had far too low a bar, routing things back to the ticket queue that a Dev could just fix in the PR already open. Rewrote the bullet above to default to blocking; a ticket is now the exception, not the default outcome of a code-review pass. - The rewritten rule still had an escape hatch wide enough to walk right back through it. 2026-09-13 (Kai orchestrator session): checking tickets filed genuinely after the 2026-09-11 rewrite (commit dd28221, merged 06:48:49 UTC that day) — #724 and #726 are the same pre-rewrite motivating evidence the entry above already covers (created 2026-09-10 05:18 and 2026-09-11 05:35 UTC, both before the rewrite landed; an earlier version of this entry double-counted them as new post-rewrite failures, caught in Bob's prompt-QA review of PR #742) — the real post-rewrite record is #732 (Ash's own QA-round finding on PR #730, cited "low priority, narrow, contrived repro," not one of the three named exceptions) vs. #737 (PR #736, correctly named the cross-cutting exception — 6+ untouched call sites — and was legitimately ticketed). One-for-two is thinner evidence than "the ratio hadn't improved," but #732 alone still shows the soft wording being walked straight through: the 2026-09-11 rewrite described the right default but still let "non-blocking" stand on its own as a reason, with nothing forcing a session to check its finding against the three exceptions before reaching for gh issue create. Tightened further: a ticket now requires the PR comment/ticket body to name which of the three bars it clears, and "narrow"/"low priority"/"not blocking" are called out explicitly as descriptions, not exceptions, so there's no unnamed middle ground to file into. Also folded in the user's efficiency framing directly into the rule's own rationale (a new ticket cycle means a Dev reconstructing context from scratch, which a same-round send-back skips) so the "why" travels with the rule instead of living only in this retro log. - Naming a bar isn't the same as clearing it — bar 1's "maintainer/product decision" framing was itself soft enough to absorb an ordinary engineering choice. 2026-09-14 (Kai orchestrator session): #751 (PR #750's /code-review, issue #366) satisfied the 2026-09-13 tightening's literal requirement — it named bar 1 explicitly ("fixing it well needs a design decision, not a mechanical fix"). But the ticket's own "options to weigh" were a cheap key-set-parity test, a cross-reference comment, or a fuller doc-generation refactor — ordinary engineering picks a Dev makes routinely in the course of a PR, not a call requiring maintainer authority. Contrast a real bar-1 case like #519 (response-cap re-evaluation, routed to Joy for an actual architecture decision that genuinely needed ownership beyond the PR at hand) — #751 had no equivalent blocker; any of its three options could have shipped in the same round. Command decision: bar 1 now excludes "multiple viable implementation approaches exist" as qualifying evidence on its own, and adds a concrete test — would sending this back right now let the Dev just pick a reasonable option and ship it? If yes, it's not bar 1. User's call on disposition: let #751 ride as already filed rather than rework or close it retroactively — this fixes the rule for the next case, not a re-litigation of this one. - Search open issues before filing any ticket, not just fixture-pollution ones. PR #800, 2026-09-24: round 2 filed #806 five minutes after #805 covered the same gap, and Kai closed #806 as a duplicate. Nothing required searching outside the fixture case. Command decision: the search-before-filing step in the ticket-exception block above. - A one-line fix for a pre-existing defect cost three extra rounds. PR #867 (#833) round 1: the code review also flagged that the token refresh ran on the event loop. That was already true on develop, and #833 (moving the consent wait off the loop) didn't cover it. Sky sent it back as blocking under "Raise the bar." The fix moved the token load onto a thread. That changed when the lifespan raises relative to the SSE endpoint event, which caused round 2's intermittent SIGTERM hang (python-sdk#3616). It also opened a stale-read window between the thread's "consent needed" decision and the event loop, which caused the CI-flake race (failure side) and round 4's race (success side). Five rounds in total, three of them from this one send-back. Command decision: bar 4 under "Raise the bar." The user chose it over broadening the pre-existing carve-out to every defect. - The rule worked as written, and two of four follow-up tickets still shouldn't have existed, while a third skipped the rule. 2026-10-04, first PRs reviewed under decisions 1–5 (#905 for #872, #903 for #883). Round 1 sent back 7 findings on #905 and 2 on #903, all fixed in-branch, which is the behavior decision 1 asked for. Every filed ticket named a bar, and the merge check passed them. But of the four tickets the two PRs produced, two shouldn't have been filed. #908 (bar 2) was a dedup refactor: it fit "outside this diff" but had no behavior impact. #906 (bar 2) was already covered by #873's planned design, which the same-defect search didn't look for. #907 (bar 4: a real pre-existing defect whose fix changes await order) was needed. #904 was filed by the Dev at PR creation with no bar named. It does clear bar 2 (a sibling function with the same cancel race), but nothing checked it. #906 was closed into #873, and #904/#907/#908 were folded into bundle #909. User's read: "we tweaked the criteria and immediately have four new tickets." Command decision: decisions 6–8 above. Each tightening so far has closed one gap and exposed the next layer down. Decision 5's two-week measurement should count Dev-filed tickets too. - A followed rule still produced a ticket per merged PR (2026-10-11, #995). 7 of the 8 issues opened that day came from PR reviews. Since 2026-10-04, 22 of 45 new tickets were from-review (10 bundle tickets not counted). Every ticket named a bar that fit as written, so compliance wasn't the problem; bar 2's per-function boundary was. #993 (_render_block, the module and escaping helper PR #991 changed) was filed and then fixed inside #991 anyway. #994 (handle_endtag in html_parser.py, which PR #992 modifies) was the same bug QA sent back in the PR's new code that round. #990 (write_capped_result_to_disk, the helper PR #988's new local_path path writes through) was observed in that PR's own QA. Separately, Kai labeled every review ticket for v0.9.2, and #987 (docstring only) and #994 (pre-existing, only on invalid nested HTML) were moved to v0.9.3 afterward. Command decision: decisions 9 and 10. - A stale process doc isn't behavior impact (2026-10-13, PR #1006). Kit filed #1014 under bar 2: CLAUDE.md's transfer.py invariant still said every quota site renders _SA_QUOTA_ERROR, which the PR had made false. CLAUDE.md wasn't in the diff, and edits to it go through Bob's prompt-QA gate, so "outside the diff" fit. The value test didn't. Stale agent guidance changes no tool output, and the argued path (an agent "fixing" a site back) was speculative. Kai caught it after merge, and the finding line was corrected to dropped. The user kept #1014 open for other reasons: it now carries the GHSA-hx2f _safe_local_dest CLAUDE.md rule. Command decision: bar 2's value test in qa.md step 3 now names this case, limited to the Bob-gated files (root CLAUDE.md, .claude/team-roles/*.md, .claude/commands/*.md) so the drop can't stretch to user-facing or generated docs (Bob's prompt-QA round 1). Open question, not decided here: a PR that makes a CLAUDE.md invariant false has no bar that tracks the fix.