Skip to content

vscode: resolve state on review comments + builder workflow for signalling addressed (#1055 follow-up) #1131

Description

@amrmelsayed

Problem

Once #1055 ships format v2 with stable IDs, review comments can carry a resolved: bool state. This unlocks the reviewer UX where an architect can walk through 15 comments and see which are addressed vs still outstanding at a glance — instead of re-reading everything from scratch.

But adding the flag is the easy part. The hard question is: who sets it, when, and how does the builder participate?

Today (verified against codev/roles/builder.md, codev/protocols/pir/prompts/implement.md and equivalents): the builder isn't instructed to handle <!-- REVIEW: --> markers at all. The only feedback channels named in the implement-phase prompt are git diff MERGE_BASE, gh issue view --comments, and afx send queue messages. The architect can add comments via the preview +, but nothing in the protocol tells the builder to check for them, address them, or signal addressed. Today's flow works by accident of Claude being generally competent, not by design.

Adding resolve state without also addressing this gap means the flag stays unresolved forever — the architect has to come back and manually walk every comment to close the loop, which is exactly the pain resolve state was supposed to solve.

Proposed scope

Ship the resolve mechanic AND the workflow that makes it useful. Also owns the format upgrade design decision that #1055 was originally going to carry — since resolve state needs a home for a new field, this issue is where the format work is driven by a real requirement rather than speculatively.

  1. Format upgrade (plan-gate decision): the mechanism for attaching resolved state to a marker. Not predetermined — plan-gate picks between:

    • Extended-attribute v2 marker with a new tag (e.g. <!-- REVIEWv2(id=abc, @amr, resolved=false): body -->) — safe-by-design for cross-host consumers (old codev-core parsers only match REVIEW(, so REVIEWv2 markers are invisible rather than corrupted). Requires migrating existing REVIEW markers on write, or supporting both tags in parallel indefinitely.
    • Companion marker on a following line (e.g. <!-- REVIEW(@amr): body --> unchanged, then <!-- RESOLVED(@amr, 2026-07-02): fixed in commit abc123 --> beneath it). Old parsers see the functional REVIEW marker unchanged and ignore the sibling; new parsers pair them up. Fully backward-compatible with no marker migration.
    • Sidecar file (per-artifact .review-state.json alongside the .md). Cleanest separation of "prose content" from "review metadata" but adds a second file to think about and to keep in sync with markers.

    Cross-host compatibility (the dashboard's bundled codev-core codec picking up files authored by newer versions) is a first-order concern in choosing here — see the discussion that spun this out of vscode: edit + preview-side delete on review comments (make the markdown reviewer functional) #1055.

  2. Toggle actions: resolve / unresolve on both surfaces (editor Comments-API and preview cards).

  3. Rendering: resolved markers render dimmer in preview cards + right-edge minimap (vscode: markdown preview marker-aware features — inline REVIEW rendering + right-edge marker minimap #863) dots; vscode: review summary webview — bird's-eye list of all REVIEW markers in spec/plan files with click-to-jump #860 summary filter picks up an "unresolved only" mode.

  4. Builder workflow instruction: add a section to codev/roles/builder.md (and skeleton copy) plus a step in codev/protocols/pir/prompts/implement.md (+ SPIR / AIR equivalents) telling the builder to:

    • Check for <!-- REVIEW: --> markers on plan / spec / review files during implement phase
    • Flip resolved=true when the underlying feedback is addressed
    • Leave unresolved with a note in the builder thread if it can't or won't address it (needs architect input, disagrees with framing)
  5. Editor Comments-API integration: wire CommentThread.state = 'unresolved' | 'resolved' from the marker field so VS Code's native comment surface renders the state.

Plan-gate decisions

  • Model A (architect-only resolve) vs Model B (builder-can-resolve) vs hybrid. Recommendation: Model B for Codev's autonomous flow (builders signal handoff, architect verifies at dev-gate). Model A requires architect round-trip that defeats the purpose in autonomous PIR. Locked at plan-gate.
  • How the builder signals "can't address": leave unresolved + write reason in builder thread, or add an explicit blocked=true third state, or an inline <!-- ADDRESSED(@builder): note --> companion marker. Plan-gate bikeshed.
  • Trust boundaries: what stops a builder from over-claiming resolved without addressing? Answer: architect at dev-gate spot-checks resolved markers by verifying the corresponding change actually happened. Worth naming explicitly in the role-doc so builders know verification is expected.

Depends on

Protocol

PIR. Design-sensitive across multiple surfaces:

  • Format upgrade mechanism — extended-attribute-v2 vs companion-marker vs sidecar file, with cross-host compatibility as a first-order constraint (the dashboard's codev-core version-lag risk)
  • Workflow model — architect-only vs builder-can-resolve vs hybrid
  • Role-doc changes across codev/roles/builder.md + skeleton copy
  • Protocol prompt updates across PIR / SPIR / AIR implement phases
  • VS Code Comments API state wiring (CommentThread.state)

All non-trivial. Not AIR — the mechanic is a boolean but the surrounding design work is real, with plan-gate benefit and dev-gate verification of the full loop (architect adds unresolved → builder resolves → architect verifies at dev-gate).

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/vscodeArea: VS Code extension

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions