feat: Add review-fix-round plumbing: /review route, board_get_feature tool, in_review requeue#99
Conversation
There was a problem hiding this comment.
QA panel review — WARN
code-review-structural · head 72505e6116a7 · formal
[review-synthesizer completed: workflow code-review-structural:report]
Prose Brief
Overall risk: low. The diff touches store.py with a depends_on projection for the board feature endpoint. The sole confirmed finding is a semantic inconsistency — not a crash, data loss, or security issue.
Fix-first item: Add a status != "closed" filter to the depends_on list comprehension in store.py (~line 1112) so it matches the existing dag_blocked logic (which already filters closed edges on the same deps list), aligning the data with the field comment "edges this feature waits on." Alternatively, if closed edges are intentionally included for audit/history purposes, revise the comment and add a per-entry status qualifier so downstream agents can distinguish active blockers from resolved history without cross-referencing dag_blocked.
Panel disagreement: None — a single finding, unanimously confirmed by verification.
Verification change: The verifier confirmed the finding with four concrete evidence checks; no severity re-grade was needed.
Gaps: Only one file (store.py) was flagged. If the PR also touched other production files or tests beyond what this finding covers, they went under-read — the structural pass (code-review-structural:verify) focused narrowly on this one issue.
[
{
"file": "store.py",
"line": 1100,
"severity": "minor",
"category": "correctness",
"claim": "`depends_on` includes closed/merged dependency edges alongside active ones, but the field comment says 'edges this feature waits on' — agents reading `board_get_feature` cannot distinguish active blockers from resolved ones without cross-referencing `dag_blocked`.",
"evidence": "`depends_on = [d.get(\"id\") for d in deps if d.get(\"dependency_type\") == \"blocks\" and d.get(\"id\")]` has no `status != \"closed\"` filter, unlike the `dag_blocked` computation on the same `deps` list which correctly uses `d.get(\"status\") != \"closed\"`. The test `test_depends_on_projection_lists_blocks_edges` confirms both open (`bd-a`) and closed (`bd-b`) edges are returned.",
"verdict": "confirmed",
"note": "Code (diff store.py ~L1112) has no status filter on depends_on; dag_blocked on same deps does filter status!='closed'; test asserts both open+closed in output; comment says 'edges this feature waits on' which closed edges no longer are — semantic inconsistency is real."
}
]|
Operator adjudication of the WARN (brain-ruled — as |
There was a problem hiding this comment.
Promoting the WARN verdict for head 72505e6116a7: all checks terminal-green, zero unresolved review threads. (approve-on-green)
Open findings carried by this approval — non-blocking, but they did not go away:
- minor
store.py:1100—depends_onincludes closed/merged dependency edges alongside active ones, but the field comment says 'edges this feature waits on' — agents readingboard_get_featurecannot distinguish active bloc
Approving a WARN does not resolve its findings (issue #22).
… tool, in_review requeue
There was a problem hiding this comment.
QA panel review — PASS
code-review-structural · head 7f1f11200ee2 · formal
[review-synthesizer completed: workflow code-review-structural:report]
Dispositions
No prior blocker or major findings exist that require a mandatory disposition block. The sole prior (round 1, store.py:1100, severity minor) was confirmed addressed — both depends_on and open_depends_on are now projected in _project and surfaced in board_get_feature, resolving the ambiguity about active vs. resolved dependency edges.
Prose Brief
Overall risk: Low. The change exposes already-stored dependency data through board_get_feature's projection layer — no new mutations, no logic shifts, no state-machine changes. All four finders converged on zero defects, and the structural pass returned nothing. The prior round's sole concern (minor: depends_on mixing resolved and active edges) is resolved in the current diff via the new open_depends_on field.
Fix-first item: None — the panel found nothing to fix.
Panel disagreement: None. Unanimous clean pass across LLM finders and the structural engine. The verifier confirmed zero findings survive.
What verification changed: The verifier confirmed the prior minor was addressed and found no new defects to surface. The empty array stands.
Coverage gap / structural pass note: No structural engine (protopatch) findings were present in the input — either the pass was skipped or returned zero items. If store.py is the heavily-changed file here, zero findings across all lanes on a production-code change is unusual and worth a second human look, but the panel is consistent.
[]
Summary
Add review-fix-round plumbing: /review route, board_get_feature tool, in_review requeue (
bd-171)See the diff for details.
Fixes #98