Skip to content

feat(kanban): full-review coverage gate on request_changes (t_164f0178) - #999

Merged
Kyzcreig merged 4 commits into
mainfrom
feat/review-coverage-gate
Sep 25, 2026
Merged

feat(kanban): full-review coverage gate on request_changes (t_164f0178)#999
Kyzcreig merged 4 commits into
mainfrom
feat/review-coverage-gate

Conversation

@Kyzcreig

@Kyzcreig Kyzcreig commented Sep 24, 2026 •
edited by blacksmith-sh Bot
Loading

Copy link
Copy Markdown
Collaborator

Card t_164f0178 (Ace ruling 2026-09-24): a review round must grade the whole deliverable and report ALL findings.

Gate — kb.request_changes refuses unless the current review run has a review_coverage: {json} comment:

  • lenses: contract, execution, cross-vendor, mutation — each done or n/a: <applicability reason>; 'cannot/could not/unavailable/failed' reasons are refused → must use kanban_block(kind=capability)
  • findings ≥ 1, items list matching the count
  • review_minutes (int ≥ 0), battery (attachment name; seeded/none-first-round only allowed on first round), batch_id
  • Comments from prior runs cannot authorize the current round. Refusal names the missing field.

Class sweep (review → implementer): tool, CLI and gateway /kanban all go through kb.request_changes. The legacy reopen_review_task bypass (CLI reopen-review, dashboard single + bulk drag) is retired and refuses (409 on dashboard). CLI adds --coverage and claim --review so human-only boards still have a path. Approval (complete) and block(kind=capability) unaffected.

Brief: KANBAN_GUIDANCE, tool schema and bundled sdlc-review skill state the gate + one delegate_task batch. Argus harness untouched.

Tests: new test_kanban_review_coverage_gate.py (18) — real DB persistence through tool handler, run_slash, and a real python -m hermes_cli.main kanban request-changes subprocess: 3 lenses refused, 4+findings accepted, capability block + approval unaffected, stale-round and re-review battery rules. Neighbor kanban suites: 411 passed; 1 failure (test_review_tools_are_gated_and_visible_to_kanban_workers, ModuleNotFoundError acp) reproduces identically on base. Mutation sweep over the parser: 10/10 mutants killed.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@Kyzcreig Kyzcreig mentioned this pull request Sep 24, 2026
Closed
Kyzcreig added a commit that referenced this pull request Sep 25, 2026
@Kyzcreig
fix(kanban): review-coverage gate r3 — close dashboard review exits, …
ceb398a 
…harden n/a + findings parsing

Addresses all 7 findings from Apollo review r3 on #999:
- F1: _set_status_direct refuses any move out of review except
  done/blocked/archived (inside the write txn); PATCH and bulk pre-check
  the same predicate for a 409. _reopen_if_review deleted.
- F2: n/a reasons are NFKC-normalized, zero-width/control chars stripped,
  and inability phrases matched anywhere (could not/cannot/unable/missing/
  not available/no ... tool/exhausted/timed out, ...).
- F3: test helpers and gate fixtures build lenses from
  REQUIRED_REVIEW_LENSES; SKILL.md example lens set == schema is tested.
- F4: newest current-run comment that parses to a JSON object wins.
- F5: items need 3+ visible chars incl. a letter/digit; review_minutes 0..1440.
- F6: lens keys and states casefolded.
- F7: reopen_review_task drops the home-session guard (no "allowed" line),
  CLI reopen-review always refuses; review_reopened documented as historical.

Verified: gate + skill + dashboard + requeue-kinds tests 126 passed;
kanban neighbours 342 passed (3 fails were worker HERMES_SESSION_ID env leak
and the override-set change, both fixed/pass with env stripped);
13/13 new parser/dashboard mutants killed.
@Kyzcreig
Kyzcreig force-pushed the feat/review-coverage-gate branch from 2ad6249 to ceb398a Compare September 25, 2026 12:10
Apollo and others added 4 commits September 25, 2026 05:55
Apollo @Kyzcreig
feat(kanban): full-review coverage gate on request_changes
482cd7c 
request_changes now refuses unless the current review run carries a
review_coverage JSON comment: all four lenses (contract, execution,
cross-vendor, mutation) done or 'n/a: <applicability reason>', findings>=1
with matching items, review_minutes, battery (named attachment on re-review),
and the delegate batch_id. Inability to run a lens must be
kanban_block(kind=capability). Refusal names the missing field.

Class sweep of review->implementer entry points: tool, CLI and gateway
/kanban all route via kb.request_changes; the legacy reopen_review_task
bypass (CLI reopen-review, dashboard single/bulk drag) is retired and
refuses. CLI gains --coverage and claim --review for human-only boards.
KANBAN_GUIDANCE, tool schema and bundled sdlc-review state the gate.

Mutation sweep: 10/10 parser mutants killed.
Apollo @Kyzcreig
fix(kanban): review-coverage gate r2 — schema-sourced lens list, n/a …
56d330f 
…opener grading, backtick form, optional battery

- Lens list lives in hermes_cli/kanban_review_schema.py; gate, tool description
  and worker prompt all read it (operator steer: lens set is being revised).
- n/a grades only the OPENING of the reason for an inability report, so
  'vendors cannot differ' is accepted and 'skipped'/'ran out of time' refused.
- Accept the documented inline-code form of the review_coverage line.
- battery optional (CI owns suites); nonempty string when given.
- Tests: one refusing test per surviving parser mutant, both n/a directions,
  verbatim SKILL.md example; rewrite test_sdlc_review_skill lens test to the
  four-lens/coverage/capability-block property.
@Kyzcreig
fix(kanban): review-coverage gate r3 — close dashboard review exits, …
a4adc0f 
…harden n/a + findings parsing

Addresses all 7 findings from Apollo review r3 on #999:
- F1: _set_status_direct refuses any move out of review except
  done/blocked/archived (inside the write txn); PATCH and bulk pre-check
  the same predicate for a 409. _reopen_if_review deleted.
- F2: n/a reasons are NFKC-normalized, zero-width/control chars stripped,
  and inability phrases matched anywhere (could not/cannot/unable/missing/
  not available/no ... tool/exhausted/timed out, ...).
- F3: test helpers and gate fixtures build lenses from
  REQUIRED_REVIEW_LENSES; SKILL.md example lens set == schema is tested.
- F4: newest current-run comment that parses to a JSON object wins.
- F5: items need 3+ visible chars incl. a letter/digit; review_minutes 0..1440.
- F6: lens keys and states casefolded.
- F7: reopen_review_task drops the home-session guard (no "allowed" line),
  CLI reopen-review always refuses; review_reopened documented as historical.

Verified: gate + skill + dashboard + requeue-kinds tests 126 passed;
kanban neighbours 342 passed (3 fails were worker HERMES_SESSION_ID env leak
and the override-set change, both fixed/pass with env stripped);
13/13 new parser/dashboard mutants killed.
@Kyzcreig
test(kanban): load-gates round-cap fixture posts review coverage befo…
becdee4 
…re request_changes

test_kanban_review_load_gates.py landed on main (53d766b) after r3's rebase and
calls bare kb.request_changes, which the coverage gate now refuses (CI slice 10/16:
3 failures at :141). Route _one_round through tests.kanban_review_helpers.
@Kyzcreig
Kyzcreig force-pushed the feat/review-coverage-gate branch from ceb398a to becdee4 Compare September 25, 2026 12:57
@Kyzcreig Kyzcreig mentioned this pull request Sep 25, 2026
Merged
@Kyzcreig

Kyzcreig commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 merged-by: apollo · lane: discord · gate: BYPASS: FR paused 09-22; gate=CI green + Apollo review · why: t_164f0178 r3: all 7 Apollo r3 findings fixed and verified by grep on head becdee4 (F1 review-exit guard owned by _set_status_direct + single/bulk tests; F2 NFKC+zero-width strip and inability phrases matched anywhere; F3 schema-driven fixtures/skill test; F4 newest VALID coverage; F5 items/minutes bounds; F6 casefold; F7 dead reopen routing removed). CI 40/40 green; local 113/114 with the one red identical on base (UNHOMED-card policy in this home, not the PR); 0 conflicts with main; reviewed by Apollo 06:5x PT

@Kyzcreig
Kyzcreig added this pull request to the merge queue Sep 25, 2026
@Kyzcreig
Kyzcreig removed this pull request from the merge queue due to a manual request Sep 25, 2026
@Kyzcreig
Kyzcreig added this pull request to the merge queue Sep 25, 2026
@Kyzcreig
Kyzcreig removed this pull request from the merge queue due to a manual request Sep 25, 2026
@Kyzcreig

Kyzcreig commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 merged-by: apollo · lane: boil-ocean · gate: BYPASS: FR PAUSED by Ace ruling 2026-09-22 (state/fleetreview-pause-20260922.md); Apollo-reviewed lands via bypass · why: re-enqueue: prior merge_group CI run hit startup_failure (0 jobs) and wedged the queue head in AWAITING_CHECKS; content unchanged

@Kyzcreig
Kyzcreig added this pull request to the merge queue Sep 25, 2026
@Kyzcreig
Kyzcreig removed this pull request from the merge queue due to a manual request Sep 25, 2026
@Kyzcreig
Kyzcreig added this pull request to the merge queue Sep 25, 2026
Merged via the queue into main with commit ecaddf6 Sep 25, 2026
104 of 107 checks passed
@Kyzcreig
Kyzcreig deleted the feat/review-coverage-gate branch September 25, 2026 18:48
@Kyzcreig Kyzcreig added the fleetreview:post-merge Ask FleetReview to review this MERGED pull (merge commit vs first parent) label Sep 25, 2026
Kyzcreig added a commit that referenced this pull request Sep 25, 2026
@Kyzcreig Apollo
fix(kanban): parked-review send-back carries its coverage onto the ru…
d8235da 
…n it opens (t_f31e2ff3)

Rebased onto #999 (review-coverage gate, becdee4). A card parked in
review has no run, so no review_coverage comment can carry the current
run_id; with #999's gate every human/Apollo send-back was refused.

request_changes(claimer=..., coverage=...):
- parked review + claimer: refuse with the gate's own message when no
  coverage is given, BEFORE opening a run (no claim churn); otherwise open
  the review run via _open_review_run (same claimed(source_status=review)
  event as the dispatched reviewer), record coverage as a comment bound to
  the new run, run the #999 gate, request changes -- one write_txn. Any
  refusal after the claim rolls back claim + comment.
- coverage is recorded on whichever run is closed (also an active review
  run), so the CLI --coverage pre-post from #999 now routes through here.
- comment journal entry written only after commit.

Surfaces: CLI request-changes --coverage passes through; kanban_request_changes
tool gains optional `coverage`; dashboard gains POST
/tasks/{id}/request-changes (review->ready status writes stay refused per #999).

Verified: test_kanban_review_sendback.py + test_kanban_review_coverage_gate.py
77 passed (py3.11 venv, via test-gate). Depends on #999.
@ang-prism

ang-prism Bot commented Sep 26, 2026

Copy link
Copy Markdown

FleetReview

FleetReview's daily member-call budget is spent (600/600 for 2026-09-26 UTC); review skipped.

FleetReview · reviewKind: skipped-budget

Kyzcreig added a commit that referenced this pull request Sep 26, 2026
@Kyzcreig @ang-fleet-workers
fix(kanban): parked-review send-back carries its coverage onto the ru…
2e9421d 
…n it opens (t_f31e2ff3)

Rebased onto #999 (review-coverage gate, becdee4). A card parked in
review has no run, so no review_coverage comment can carry the current
run_id; with #999's gate every human/Apollo send-back was refused.

request_changes(claimer=..., coverage=...):
- parked review + claimer: refuse with the gate's own message when no
  coverage is given, BEFORE opening a run (no claim churn); otherwise open
  the review run via _open_review_run (same claimed(source_status=review)
  event as the dispatched reviewer), record coverage as a comment bound to
  the new run, run the #999 gate, request changes -- one write_txn. Any
  refusal after the claim rolls back claim + comment.
- coverage is recorded on whichever run is closed (also an active review
  run), so the CLI --coverage pre-post from #999 now routes through here.
- comment journal entry written only after commit.

Surfaces: CLI request-changes --coverage passes through; kanban_request_changes
tool gains optional `coverage`; dashboard gains POST
/tasks/{id}/request-changes (review->ready status writes stay refused per #999).

Verified: test_kanban_review_sendback.py + test_kanban_review_coverage_gate.py
77 passed (py3.11 venv, via test-gate). Depends on #999.
Kyzcreig added a commit that referenced this pull request Sep 26, 2026
@Kyzcreig @ang-fleet-workers
fix(kanban): parked-review send-back carries its coverage onto the ru…
96a904e 
…n it opens (t_f31e2ff3)

Rebased onto #999 (review-coverage gate, becdee4). A card parked in
review has no run, so no review_coverage comment can carry the current
run_id; with #999's gate every human/Apollo send-back was refused.

request_changes(claimer=..., coverage=...):
- parked review + claimer: refuse with the gate's own message when no
  coverage is given, BEFORE opening a run (no claim churn); otherwise open
  the review run via _open_review_run (same claimed(source_status=review)
  event as the dispatched reviewer), record coverage as a comment bound to
  the new run, run the #999 gate, request changes -- one write_txn. Any
  refusal after the claim rolls back claim + comment.
- coverage is recorded on whichever run is closed (also an active review
  run), so the CLI --coverage pre-post from #999 now routes through here.
- comment journal entry written only after commit.

Surfaces: CLI request-changes --coverage passes through; kanban_request_changes
tool gains optional `coverage`; dashboard gains POST
/tasks/{id}/request-changes (review->ready status writes stay refused per #999).

Verified: test_kanban_review_sendback.py + test_kanban_review_coverage_gate.py
77 passed (py3.11 venv, via test-gate). Depends on #999.
This was referenced Sep 27, 2026
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

fleetreview:post-merge Ask FleetReview to review this MERGED pull (merge commit vs first parent)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Kyzcreig