Skip to content

feat: Add review-fix-round plumbing: /review route, board_get_feature tool, in_review requeue#99

Merged
mabry1985 merged 1 commit into
mainfrom
feat/bd-171
Jul 23, 2026
Merged

feat: Add review-fix-round plumbing: /review route, board_get_feature tool, in_review requeue#99
mabry1985 merged 1 commit into
mainfrom
feat/bd-171

Conversation

@mabry1985

@mabry1985 mabry1985 commented Jul 23, 2026

Copy link
Copy Markdown
Member

Summary

Add review-fix-round plumbing: /review route, board_get_feature tool, in_review requeue (bd-171)

See the diff for details.

Fixes #98

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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."
  }
]

@mabry1985

Copy link
Copy Markdown
Member Author

Operator adjudication of the WARN (brain-ruled — as board_get_feature's primary consumer): fix-now, shape (b): keep depends_on as the full historical ledger, add open_depends_on with live blockers only (the actionable signal), and align the field docs. Requeueing the surgical round.

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:1100depends_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 bloc

Approving a WARN does not resolve its findings (issue #22).

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


[]

@mabry1985
mabry1985 merged commit e176328 into main Jul 23, 2026
1 check passed
@mabry1985
mabry1985 deleted the feat/bd-171 branch July 23, 2026 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant