Uh oh!
There was an error while loading. Please reload this page.
Fix #1606: revive Stream Deck Review dials for BUGFIX/AIR phases + honest none-mode label - #1607
Merged
Conversation
…t none-mode label The Stream Deck Review dials (Files/Headings, Changes/Blocks) silently no-op for a BUGFIX builder's whole working life (investigate, fix) and for AIR during the pr phase, because phaseArtifactVerb's hardcoded SPIR/PIR phase vocabulary left those ids unrecognized -> reviewMode resolved to 'none' and every dial gesture no-opped. Compounding it, the none-mode strip rendered the diff label (Files · send) while inert -- a gesture that lied about what it did. - reviewMode: recognize investigate/fix/pr as diff mode via a registered DIFF_REVIEW_PHASES set with a GATE_LABELS-style registration note (protocol.json phase metadata carries no review-mode field; the SDK's PHASE_TO_STAGE folds investigate->plan, wrong here, and drives the VSCode tree). phaseArtifactVerb is left untouched so the Builder Action key (open-terminal) and Zoom (view-diff) keep their behavior; only the review dials gain the extra vocabulary. - ReviewNav.renderTo: none mode now renders "No review target" instead of borrowing the diff label. - Tests: reviewMode coverage for the BUGFIX/AIR phase ids and init/no-status; a phaseArtifactVerb guard documenting the deliberate split; a behavioral test (investigate dials drive the diff) and a legibility test (honest none label).
…revived dial vocabulary Addresses CMAP (claude) review comments on PR #1607: - README.md: the Review-dial bullets described the diff vocabulary as "implement / review, or blocked at dev-approval / pr" and never mentioned the inert state. Updated to include the BUGFIX/AIR investigate/fix/pr phases and the honest "No review target" strip, so the doc that ships to the Marketplace matches the fixed behavior. - DIFF_REVIEW_PHASES note: `investigate` is registered by name, not owned by BUGFIX (RESEARCH uses it too, and its worktree changes review as a diff). Corrected the comment's attribution. No behavior change. MAINTAIN/EXPERIMENT/SPIKE phases still resolving to none is left for an architect scope decision (out of #1606's investigate/fix/pr scope); the honest label already stops them from lying.
… ruling)
Owner ruling 2026-09-04: only the `pr` phase should drive the review dials to diff
mode. BUGFIX `investigate`/`fix` are work-in-progress (partial or unwritten diff),
so they are not a review target and stay `none` — inert, but now with the honest
`No review target` label instead of the old `Files · send` lie.
- DIFF_REVIEW_PHASES = {pr}; investigate/fix removed and documented as deliberately
excluded so they are not re-added without an owner ruling.
- Tests: pr phase -> diff (behavioral + unit); investigate/fix -> none (behavioral +
unit); guard and none-label tests updated. 248 pass; tsc + build clean.
- README + PR body updated to pr-only scope.Uh oh!
There was an error while loading. Please reload this page.
amrmelsayed added a commit
that referenced
this pull request
Sep 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Stream Deck Review dials (Files/Headings, Changes/Blocks) silently no-op for an AIR builder during the
prphase and for a BUGFIX builder in theprphase (until the pr gate is requested), then come alive only when blocked at the gate — read in live use as "the diff dials sometimes work and sometimes not". This revives them for theprphase and makes the inert (none) state tell the truth on the strip instead of borrowing the diff label.Fixes#1606
Root Cause
phaseArtifactVerb(apps/streamdeck/src/actions.ts) resolves dial mode from a vocabulary hardcoded to SPIR/PIR (phasesspecify|plan|implement|review|verify, gatesspec-approval|plan-approval|dev-approval|pr|verify-approval). It matches on the exact phase-id string, so a phase whose diff-producing work is named differently falls through:investigate/fix/pr) and AIR (implement/pr): theprphase id is unrecognized, soreviewModeresolves tononebetween entering the phase and the pr gate firing (blockedGate === 'pr'was already handled).In
nonemode everyReviewNavgesture no-ops. Compounding it,ReviewNav.renderTohad only canvas/diff label branches, sononerendered the diff label (Files · send) while every gesture was inert — a gesture that lied about what it does.Fix
prphase.reviewModenow maps theprphase to diff mode via a registeredDIFF_REVIEW_PHASESset, applied after the existing verb-based canvas/diff resolution so gates still win. By theprphase the work exists as a finished diff (owner ruling 2026-09-04).investigate/fixare deliberately not diffable (owner ruling 2026-09-04): their diff is partial or not yet written, so they are not a review target. They staynone— butReviewNav.renderTonow rendersNo review targetinnonemode instead of borrowing the diff label.init/no-status builders arenonetoo, with the same honest label.Scope (owner ruling 2026-09-04)
Only the
prphase is revived;investigate/fixare work-in-progress and stay inert. The value of the fix there is the honest label — the dials no longer claimFiles · sendwhile doing nothing. The broader question of MAINTAIN/EXPERIMENT/SPIKE phases (which also produce diffs and still resolve tonone) is tracked in follow-up #1609, not this PR.Trade-off note (metadata vs. registered list)
The issue preferred deriving mode from
protocol.jsonphase metadata over a hardcoded name list. That path is not currently expressible:protocol.jsonphases carry onlyid/name/type/steps/transition/gate— no field encodes review mode — and the SDK'sPHASE_TO_STAGEdrives the VSCode Builders tree with different semantics, so it can't be repurposed. Per the issue's documented fallback, I registered theprphase inreviewModewith aGATE_LABELS-style registration note.phaseArtifactVerbis deliberately left untouched, so the Builder Action key (open-terminal) and the Zoom dial (view-diff) keep their behavior; a guard test documents that split.Shipping
apps/streamdeckchange → needs a plugin version bump + Elgato Marketplace resubmission (~4–10 working days). Per the architect the release is held: this merges tomainwith no version bump and ships later in a batched resubmission. Hardware verification runs on the architect's side via streamdeck link.Test Plan
reviewMode:prphase → diff;investigate/fix→ none (WIP);init/no-status → none. A behavioral test (a BUGFIXpr-phase builder's dials emit diff verbs) and its inverse (aninvestigatebuilder's dials stay inert). A legibility test (none-mode strip saysNo review target, notFiles · send). AphaseArtifactVerbguard for the deliberate split. Verified the behavioral/mode/label tests fail without the source fix and pass with it.npm run build, esbuild +tsc --noEmitclean)vitest run— 248 passed)