diff --git a/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/.openspec.yaml b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/.openspec.yaml new file mode 100644 index 0000000..9323e24 --- /dev/null +++ b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-04-24 diff --git a/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/approval-summary.md b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/approval-summary.md new file mode 100644 index 0000000..740b334 --- /dev/null +++ b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/approval-summary.md @@ -0,0 +1,92 @@ +# Approval Summary: extend-specflow-watch-with-review-ledger-digests-and-subagent-activity + +**Generated**: 2026-04-25T01:53:24Z +**Branch**: extend-specflow-watch-with-review-ledger-digests-and-subagent-activity +**Status**: ⚠️ 1 unresolved high + +## What Changed + +``` + src/bin/specflow-watch.ts | 21 ++ + src/lib/specflow-watch/artifact-readers.ts | 289 ++++++++++++++- + src/lib/watch-renderer/index.ts | 7 + + src/lib/watch-renderer/model.ts | 237 +++++++++++- + src/lib/watch-renderer/render.ts | 76 +++- + src/tests/specflow-watch-integration.test.ts | 153 ++++++++ + src/tests/specflow-watch-readers.test.ts | 230 ++++++++++++ + src/tests/watch-renderer.test.ts | 531 ++++++++++++++++++++++++++- + 8 files changed, 1537 insertions(+), 7 deletions(-) +``` + +## Files Touched + +``` +src/bin/specflow-watch.ts +src/lib/specflow-watch/artifact-readers.ts +src/lib/watch-renderer/index.ts +src/lib/watch-renderer/model.ts +src/lib/watch-renderer/render.ts +src/tests/specflow-watch-integration.test.ts +src/tests/specflow-watch-readers.test.ts +src/tests/watch-renderer.test.ts +``` + +## Review Loop Summary + +### Design Review + +| Metric | Count | +|--------------------|-------| +| Initial high | 1 | +| Resolved high | 1 | +| Unresolved high | 0 | +| New high (later) | 1 | +| Total rounds | 5 | + +### Impl Review + +| Metric | Count | +|--------------------|-------| +| Initial high | 1 | +| Resolved high | 1 | +| Unresolved high | 1 | +| New high (later) | 1 | +| Total rounds | 3 | + +## Proposal Coverage + +Spec acceptance scenarios from `specs/realtime-progress-ui/spec.md`: + +| # | Criterion (summary) | Covered? | Mapped Files | +|---|---------------------|----------|--------------| +| 1 | Digest renders under snapshot progress during apply_review | Yes | src/lib/watch-renderer/model.ts, src/lib/watch-renderer/render.ts, src/bin/specflow-watch.ts | +| 2 | Severity breakdown only counts open findings | Yes | src/lib/watch-renderer/model.ts, src/tests/watch-renderer.test.ts | +| 3 | Open findings list ranks by severity → latest_round → id | Yes | src/lib/watch-renderer/model.ts (rankOpenFindings), src/tests/watch-renderer.test.ts | +| 4 | Digest remains visible on apply_ready after review completes | Yes | src/lib/specflow-watch/artifact-readers.ts (selectActiveReviewLedger), src/tests/specflow-watch-integration.test.ts | +| 5 | Digest is suppressed on non-review phases (hidden state) | Yes | src/lib/watch-renderer/model.ts, src/tests/watch-renderer.test.ts | +| 6 | Latest round summary missing elides the line | Yes | src/lib/watch-renderer/model.ts (buildSummaryState returns absent), src/lib/watch-renderer/render.ts | +| 7 | Findings list collapses below 80 columns | Yes | src/lib/watch-renderer/render.ts (NARROW_TERMINAL_THRESHOLD), src/tests/watch-renderer.test.ts | +| 8 | Non-findings digest lines truncate with ellipsis below 80 cols | Yes | src/lib/watch-renderer/render.ts (ellipsizeForCols), src/tests/watch-renderer.test.ts | +| 9 | Wide terminals render the full digest | Yes | src/lib/watch-renderer/render.ts, src/tests/watch-renderer.test.ts | +| 10 | UI consumes only read-only artifacts (no writes) | Yes | src/tests/specflow-watch-import-graph.test.ts (denylist guard) | +| 11 | Review ledger path derived from change_name + active family | Yes | src/lib/specflow-watch/artifact-readers.ts (reviewLedgerPath, selectActiveReviewLedger) | +| 12 | Missing ledger shows digest placeholder | Yes | src/lib/watch-renderer/model.ts (buildDigestState placeholder branch), src/tests/watch-renderer.test.ts | +| 13 | Unreadable ledger shows inline warning | Yes | src/lib/watch-renderer/model.ts (warning branch), src/tests/watch-renderer.test.ts | +| 14 | Malformed ledger shows inline warning | Yes | src/lib/specflow-watch/artifact-readers.ts (validateReviewLedgerSchema), src/tests/watch-renderer.test.ts | +| 15 | Empty ledger (no LedgerSnapshot entries) shows digest placeholder | Yes | src/lib/watch-renderer/model.ts (buildDigestFromLedger returns null on empty round_summaries), src/tests/watch-renderer.test.ts | + +**Coverage Rate**: 15/15 (100%) + +## Remaining Risks + +- R3-F01: Digest never renders the required latest-summary line (severity: high) — accepted_risk pending; Phase 2 work to add a compliant persisted summary source. Currently no `LedgerRoundSummary.summary` field exists in the persisted ledger; the spec scenario "Latest round summary missing elides the line" explicitly permits omission, which the implementation honors. +- R3-F02: New tests lock in the spec-violating summary omission (severity: medium) — accepted_risk; tests assert the spec-allowed omission contract. Will be replaced when persisted summary text becomes available. +- R2-F02: Staleness compared against `round_summaries.length` (severity: medium) — moot now that summary state is always `absent`; no comparison performed. + +## Human Checkpoints + +- [ ] Confirm the Phase 2 plan to add a persisted narrative summary source (e.g., extend `LedgerRoundSummary` with a free-form `summary` field, or define a watcher-readable artifact with explicit ownership). Track as a follow-up issue before users complain about the omitted `Latest summary:` line. +- [ ] Verify the new ledger reader is exercised end-to-end: run `specflow-watch` against a live design or apply review and confirm the digest section renders with decision, counts, severity, and top-3 findings as the ledger updates between rounds. +- [ ] Sanity-check the narrow-terminal collapse (run with `COLUMNS=60` or a 60-column terminal and confirm the findings list disappears while the decision/counts/severity lines remain readable). +- [ ] Review `validateReviewLedgerSchema` in `src/lib/specflow-watch/artifact-readers.ts` for any future `ReviewLedger` fields that should be required vs. optional; the current strict validator may need updating when contracts evolve. +- [ ] Confirm the import-graph guard (`src/tests/specflow-watch-import-graph.test.ts`) still asserts the read-only boundary after this change — no orchestration mutators are imported. diff --git a/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/current-phase.md b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/current-phase.md new file mode 100644 index 0000000..4f6f96f --- /dev/null +++ b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/current-phase.md @@ -0,0 +1,11 @@ +# Current Phase: extend-specflow-watch-with-review-ledger-digests-and-subagent-activity + +- Phase: fix-review +- Round: 3 +- Status: has_open_high +- Open High/Critical Findings: 1 件 — "Digest never renders the required latest-summary line" +- Actionable Findings: 2 +- Accepted Risks: none +- Latest Changes: + - (no commits yet) +- Next Recommended Action: /specflow.fix_apply diff --git a/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/design.md b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/design.md new file mode 100644 index 0000000..87f1911 --- /dev/null +++ b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/design.md @@ -0,0 +1,307 @@ +## Context + +`specflow-watch` is a standalone terminal TUI for a single specflow run. It is structured as three pure layers: + +1. **Artifact readers** — `src/lib/specflow-watch/artifact-readers.ts` exposes tolerant readers that return a tagged `ArtifactReadResult` with `kind ∈ {ok, absent, unreadable, malformed}` for each input file. +2. **Model builder** — constructs the in-memory model the renderer consumes from the reader results. Run-state is the only mandatory input; all other sources degrade gracefully. +3. **Renderer** — the TUI in `src/bin/specflow-watch.ts` renders sections (Run header, Review round, Task graph, Approval summary, Recent events), redraws on filesystem changes, and exits on `q` / `Ctrl+C`. + +The Review round section currently derives entirely from `autofix-progress-.json`. The review-ledger data — `openspec/changes//review-ledger-design.json` and `openspec/changes//review-ledger.json` — is already persisted by the review orchestration layer (`src/lib/review-ledger.ts`, types in `src/types/contracts.ts`) but is never read by the watcher. Operators therefore see round/progress counters but not the actual review outcome. + +This change is a strictly additive widening of the read model: new tolerant readers for the two ledger files and the two review-result files (summary source), a new optional digest sub-model attached to the Review section, and new renderer lines below the existing progress lines. Nothing about the run model, review-orchestration contracts, observation events, or the watcher's read-only boundary changes. + +Spec delta: `openspec/changes/extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/specs/realtime-progress-ui/spec.md` (two ADDED requirements for the digest + narrow-terminal behavior, two MODIFIED requirements extending the read-artifact contract and the degradation rules). + +## Goals / Non-Goals + +**Goals:** +- Add tolerant readers for `review-ledger-design.json` and `review-ledger.json`, matching the existing `ArtifactReadResult` contract (`ok` / `absent` / `unreadable` / `malformed`). +- Select the ledger file by current review family, reusing the existing family rule from `selectActiveAutofixPhase`. +- Build a digest sub-model from the **latest persisted ledger state** (the ledger's own "latest round" — the last `round_summaries` entry plus the ledger's open findings) that carries: decision, total/open/new/resolved counts, severity breakdown over open findings, a `SummaryState` for the latest round narrative (sourced from `review-result-.json` with round-match validation against the ledger), top-3 open findings. +- Rank the top-3 open findings by `severity` (CRITICAL/HIGH > MEDIUM > LOW — `critical` maps to the same rank as `high`), then `latest_round` DESC, then `id` ASC as deterministic tie-breaker. +- Render digest lines below the existing Review section progress lines with a compact format; auto-collapse the findings list and truncate other digest lines with `…` when the terminal is narrower than 80 columns. +- Preserve the existing snapshot-based progress rendering and all graceful-degradation rules for non-ledger sources. +- Maintain the watcher's strictly read-only boundary (no `specflow-run advance`, no file writes). + +**Non-Goals:** +- Rendering any raw ledger JSON or a full ledger browser. +- Aggregating counts across multiple rounds (digest draws from the latest round only). +- Surfacing "subagent activity" mentioned in the issue's title (explicitly out of scope for this change — a separate proposal if needed). +- Changing `ReviewLedger`, `LedgerSnapshot`, or `LedgerRoundSummary` shapes, or the review orchestration / persistence contracts. +- Modifying review-result persistence (the watcher only reads `review-result-.json` as a summary source; it does not write or alter it). +- Modifying the observation-events stream, run-state schema, or task-graph contract. +- Rewriting the existing Review round section behavior (snapshot-derived progress remains intact). +- Adding any network / daemon / IPC behavior to `specflow-watch`. + +## Decisions + +### D1. Source of the "latest" digest state is the ledger's last round_summary + open findings + +The spec references the "latest `LedgerSnapshot`". In code, `LedgerSnapshot` is an in-memory view (`src/types/contracts.ts:434`) produced by the helper `ledgerSnapshot(ledger)` (`src/lib/review-ledger.ts:750`); it is not a distinct on-disk record. The **persisted** ledger (`ReviewLedger` at `src/types/contracts.ts:393`) carries: +- `latest_decision` at the top level +- `round_summaries: readonly LedgerRoundSummary[]` — each round's counters, decision, stop_reason, etc. +- `findings: readonly ReviewFinding[]` — full finding history with `status`, `severity`, `origin_round`, `latest_round` + +The digest SHALL be built from: +- **Decision** (single source of truth): `ledger.latest_decision` is the authoritative decision field. If `ledger.latest_decision` is present, it SHALL be used unconditionally. If `ledger.latest_decision` is absent or empty, fall back to `round_summaries[len-1].decision`. If both are empty, render `Decision: (none)`. Additionally, when both `ledger.latest_decision` and `round_summaries[len-1].decision` are present and they disagree, the reader SHALL return `malformed` with a reason describing the decision parity violation — this prevents silently rendering stale or inconsistent decision data. +- **Counts (total / open / new / resolved)**: read from `round_summaries[len-1]` (the last entry represents the latest round). This matches the ledger's own persisted counters and avoids recomputing anything the persistence layer already established. +- **Severity breakdown over open findings**: filter `ledger.findings` by `status ∈ {open, new}`, then group by `severity`. Findings with `severity === "critical"` are aggregated into the `high` count — the existing review-orchestration contract treats `critical` and `high` as the same blocking tier, and the digest display uses `HIGH n | MEDIUM n | LOW n` without a separate CRITICAL column. This ensures `critical` findings are never understated or hidden. Unknown severities (not `critical`, `high`, `medium`, or `low`) are also aggregated into the `high` count as a safe default. Does **not** use `round_summaries[*].by_severity` because that dict mixes all statuses. +- **Latest summary text**: the persisted `LedgerRoundSummary` does not carry a free-form `summary` string. The narrative summary is sourced from a separate artifact: `review-result-design.json` or `review-result.json` (family-mapped, same selection rule as the ledger). The watcher SHALL add a tolerant reader for the review-result file (same `ArtifactReadResult` contract). `ReviewResultSummary` SHALL extract both `summary?: string` and `round_index?: number` from the review-result file. The model builder SHALL validate that the review-result's `round_index` matches the ledger's latest round (`round_summaries.length`) before accepting the summary as current. This prevents pairing a newer ledger state with a stale narrative from an older round. The `summaryState` field on `LedgerDigest` (see D4) uses a discriminated union to represent the outcome: + - `{ kind: "available", text: string }` — review-result is present, has a `summary` field, and its `round_index` matches the ledger's latest round (or `round_index` is absent in the review-result, in which case the match is assumed — the watcher cannot distinguish "no round_index field" from "matches" and accepts it optimistically). + - `{ kind: "stale", text: string, resultRound: number, ledgerRound: number }` — review-result has a `summary` and a `round_index`, but the `round_index` does not match the ledger's latest round. The renderer SHALL show `Latest summary (stale — round ):` followed by the text, so the operator knows the narrative may not reflect the current ledger state. + - `{ kind: "absent" }` — review-result file is absent, lacks a `summary` field, or `summary` is empty. The renderer SHALL omit the `Latest summary:` line entirely. + - `{ kind: "warning", reason: string }` — review-result file is unreadable or malformed. The renderer SHALL show `Review summary unreadable: ` as a distinct warning line, so the operator can distinguish a broken summary source from a legitimately absent one. + This satisfies the spec's "Latest round summary missing elides the line" scenario for the `absent` case, surfaces staleness explicitly for the `stale` case, and separates broken sources from missing ones for the `warning` case. +- **Top-3 open findings**: filter `ledger.findings` by `status ∈ {open, new}`, sort per the ranking rule, take the first three. + +**Alternative considered**: call `ledgerSnapshot(ledger)` from `review-ledger.ts`. Rejected for two reasons — it pulls all `round_summaries` through, which the digest doesn't need, and it adds a coupling from the watcher to orchestration helper code. The watcher should only depend on shape types (`ReviewLedger`, `LedgerRoundSummary`, `ReviewFinding`), not orchestration logic. + +### D2. Parser uses a read-only validator against the full ReviewLedger schema + +`src/lib/review-ledger.ts`'s `readLedger` does **not** fit the watcher because it has side effects (renames corrupt files to `.corrupt` paths and falls back to backups). The watcher must remain strictly read-only. + +A new reader in `artifact-readers.ts` SHALL: +1. Check `existsSync(path)`; `absent` on miss. +2. Read the file with `readFileSync`; `unreadable` on I/O error. +3. `JSON.parse`; `malformed` on parse failure. +4. Run a **watcher-local** validator that validates parsed JSON against the full `ReviewLedger` contract schema — all required fields (`feature_id`, `phase`, `findings` array with required `ReviewFinding` fields, `round_summaries` array with required `LedgerRoundSummary` fields, optional `latest_decision`). A ledger that does not conform to the full schema SHALL be returned as `malformed` with a reason describing the schema violation. This ensures schema-invalid ledgers trigger the spec-required inline warning path rather than silently rendering partial data. +5. Return `{ kind: "ok", value: ledger }` only when the full schema validates. + +The validator is side-effect-free (no file renames, no backup fallbacks) — the only difference from `validateLedger` in `review-ledger.ts` is the absence of write-side recovery logic and the tagged-result return type. + +**Alternative considered**: a loose validator scoped to only the fields the digest renders. Rejected because the spec requires the malformed-ledger warning path to activate when the ledger "does not conform to the ledger schema" — a loose validator would accept schema-invalid ledgers and hide drift. + +**Alternative considered**: reuse `parseJson` from `review-ledger.ts`. Rejected because it throws instead of returning tagged results, conflating malformed with unreadable. + +### D3. Ledger selection reuses `selectActiveAutofixPhase` family rule + +A new helper `selectActiveReviewLedger(currentPhase): "design" | "apply" | null` mirrors `selectActiveAutofixPhase` (`artifact-readers.ts:115`). The same switch values (`design_draft / design_review / design_ready` → design; `apply_draft / apply_review / apply_ready / approved` → apply; else `null`) are used so snapshot and digest stay phase-consistent. + +**Alternative considered**: derive the ledger family from the already-selected autofix phase rather than current_phase. Rejected because the two selection rules happen to be 1:1 today but are conceptually independent — keeping them parallel prevents accidental drift. + +### D4. Digest lives on the Review section model with independent snapshot and digest sub-states + +The Review section model SHALL use a composite state with **independent** sub-states for the snapshot progress layer and the ledger digest layer. Each sub-state tracks its own availability independently so that: +- snapshot-only (ledger absent/malformed) renders the progress view plus a digest placeholder or warning; +- ledger-only (snapshot absent) renders a snapshot placeholder plus the digest; +- both present renders both layers; +- mixed warning states (e.g., snapshot ok + ledger malformed) render independently. + +```ts +/** Independent state for each layer of the Review section. */ +type ReviewLayerState = + | { readonly kind: "ok"; readonly value: T } + | { readonly kind: "placeholder"; readonly message: string } + | { readonly kind: "warning"; readonly message: string } + | { readonly kind: "hidden" }; + +/** Discriminated state for the latest-round narrative summary. */ +type SummaryState = + | { readonly kind: "available"; readonly text: string } + | { readonly kind: "stale"; readonly text: string; readonly resultRound: number; readonly ledgerRound: number } + | { readonly kind: "absent" } + | { readonly kind: "warning"; readonly reason: string }; + +interface LedgerDigest { + decision: string; // "(none)" when unavailable + counts: { total: number; open: number; new_count: number; resolved: number }; + openSeverity: { high: number; medium: number; low: number }; // "critical" and unknown severities aggregated into "high" + summaryState: SummaryState; // replaces latestSummary: string | null + topOpen: readonly { // up to 3 entries; "critical" findings display as "HIGH" + severity: "HIGH" | "MEDIUM" | "LOW"; + title: string; + id: string; + }[]; +} + +interface ReviewSectionModel { + // ... existing fields (round_index, max_rounds, loop_state, etc.) + readonly snapshotState: ReviewLayerState; + readonly digestState: ReviewLayerState; +} +``` + +The renderer evaluates `snapshotState` and `digestState` independently: +- `snapshotState.kind === "ok"` → render existing progress lines. +- `snapshotState.kind === "placeholder"` → render snapshot placeholder (existing behavior). +- `snapshotState.kind === "warning"` → render snapshot warning (existing behavior). +- `snapshotState.kind === "hidden"` → render nothing for the snapshot layer. +- `digestState.kind === "ok"` → render the 4–5 digest lines plus the top-3 block. The summary line renders according to `summaryState.kind`: `available` → `Latest summary: `, `stale` → `Latest summary (stale — round ): `, `absent` → line omitted, `warning` → `Review summary unreadable: `. +- `digestState.kind === "placeholder"` → render `No review digest yet`. +- `digestState.kind === "warning"` → render `Review ledger unreadable: ` or `Review ledger malformed: `. +- `digestState.kind === "hidden"` → render nothing for the digest layer (phase is outside review families; no ledger is applicable). + +When `selectActiveReviewLedger(currentPhase)` returns `null` (non-review phase), `digestState` SHALL be `{ kind: "hidden" }`. This distinguishes "active review family with no digest yet" (`placeholder`) from "phase is outside review families" (`hidden`). The `placeholder` state is reserved for review-family phases where the ledger file is absent or has zero `round_summaries`. + +Both layers render in sequence (snapshot first, then digest) regardless of the other layer's state. This ensures that a digest warning never suppresses the snapshot view, and a snapshot placeholder never suppresses the digest. + +**Alternative considered**: a single `LedgerDigest | null` field with inline `warning` and `placeholder` flags. Rejected because it conflates snapshot and digest availability into one state path, risking suppression of the digest when the snapshot is absent or vice versa (see R1-F02). + +**Alternative considered**: a separate Review Ledger section. Rejected — the issue explicitly asks for the progress view and digest to share one section with two layers, and splitting them would break the family-aware phase visibility that the existing Review round section already handles. + +### D5. Narrow-terminal handling is a rendering-layer concern, not a model concern + +The model produces the full digest regardless of terminal width. The renderer reads `process.stdout.columns` (with a sensible default of 80 when unavailable) and applies: +- `columns < 80`: drop the `Open findings:` header and all finding rows; for other lines, truncate to `columns` characters, replacing the tail with `…` when truncation occurs. +- `columns >= 80`: render all lines in full; no truncation applied to digest lines specifically. + +Keeping width-dependent logic in the renderer means model snapshots stay stable for testing and the terminal resize path only redraws; it does not invalidate cached data. + +**Alternative considered**: compute a "narrow" digest variant in the model. Rejected — that couples the model to terminal dimensions and complicates test fixtures. + +### D6. Ranking implementation: stable sort with explicit tie-breakers + +```ts +function rankOpenFindings(findings: readonly ReviewFinding[]): readonly ReviewFinding[] { + const SEVERITY_RANK: Record = { critical: 0, high: 0, medium: 1, low: 2 }; + return [...findings] + .filter(f => f.status === "open" || f.status === "new") + .sort((a, b) => { + const sa = SEVERITY_RANK[a.severity ?? "low"] ?? 0; + const sb = SEVERITY_RANK[b.severity ?? "low"] ?? 0; + if (sa !== sb) return sa - sb; + const ra = a.latest_round ?? 0; + const rb = b.latest_round ?? 0; + if (ra !== rb) return rb - ra; // DESC + return (a.id ?? "").localeCompare(b.id ?? ""); + }); +} +``` + +`critical` maps to rank 0 (same as `high`) because the existing review-orchestration contract treats both as blocking severities. Unknown severities also default to rank 0 (the highest tier) to prevent understating unrecognized severity values. In the top-3 display, findings with `severity === "critical"` render as `HIGH` to match the digest's three-tier display format. `id` ASC uses `localeCompare` so lexicographic ordering of `RN-FNN` is deterministic across runtimes. + +### D7. Filesystem watch: watch the ledger paths alongside existing sources + +The filesystem-watch layer already watches a set of paths keyed off run + change. Two entries are appended to that set per run — the design and apply ledger paths — gated by whether the run's change directory resolves. The polling-fallback tick (≈2 s) inspects mtime/size on the same paths. Missing files are handled exclusively through the tolerant reader at redraw time; no special "file deleted" watch event is needed. + +### D8. No new observation events + +The watcher must not emit observation events (it is read-only by contract). Digest rendering fetches data on every redraw; no new event kinds are introduced. + +## Concerns + +Each concern below corresponds to a vertical slice and maps cleanly to test-layer boundaries. + +- **C-Reader** — tolerant readers for the two ledger files and the two review-result files (summary source). Resolves the "no ledger access" problem in the current watcher. Owns the `ArtifactReadResult` contract expansion and the `ArtifactReadResult` for the review-result reader. +- **C-Select** — ledger-family selection helper (`selectActiveReviewLedger`). Resolves "which ledger for which phase" consistently with the existing snapshot family rule. Owns one pure function plus tests for every phase value. +- **C-Digest-Model** — digest sub-model builder consuming a reader result and a ledger (when present). Resolves "shape of digest data exposed to renderer". Owns the `ReviewLayerState` type (including the `hidden` kind for non-review phases), `SummaryState` derivation (round-match validation, stale detection, warning vs absent discrimination), severity aggregation over open findings (including `critical` → `high` mapping), ranking (with `critical` at rank 0 alongside `high`), decision parity validation, and the independent digest state derivation (ok / placeholder / warning / hidden). Also owns the composite `ReviewSectionModel` with independent `snapshotState` and `digestState` sub-states, ensuring ledger-only, snapshot-only, hidden, and mixed warning/placeholder combinations are all representable. +- **C-Digest-Render** — renderer updates to the Review section. Resolves "compact digest below progress" UX. Owns line formatting, the narrow-terminal collapse/truncation rules, and placeholder/warning rendering. +- **C-Integration** — wire readers → model → renderer inside `src/bin/specflow-watch.ts`, plus filesystem-watch path expansion. Resolves "digest actually appears end-to-end". Owns the integration seams. + +## State / Lifecycle + +- **Canonical state** (unchanged): on-disk `review-ledger-design.json` / `review-ledger.json` files written exclusively by review orchestration. The watcher never owns or mutates this state. +- **Derived state** (new): + - `LedgerDigest` — computed per redraw from the current ledger reader result, the last `round_summaries` entry, and the open-findings slice. Cached nowhere across redraws; the model rebuilds it each cycle from the fresh reader output. + - The digest's `hidden` / `placeholder` / `warning` / normal render flags are derived state, not persisted. +- **Lifecycle boundaries**: + - Digest becomes visible as soon as the run's `current_phase` enters a review family **and** the ledger reader returns `ok` with at least one `round_summaries` entry. + - Digest remains visible during `design_ready` / `apply_ready` / `approved` (adjacent completed-family phases) per the existing family rule. + - Digest disappears when `current_phase` leaves the review families (e.g., `spec_draft`, `proposal_clarify`). + - Watcher terminal-state behavior (staying open after `run.status` transitions out of `active`) continues to apply — the final digest snapshot remains rendered using the last successful ledger read. +- **Persistence-sensitive state**: none introduced. The watcher writes no new files, no cache directory, no lock file. + +## Contracts / Interfaces + +Interfaces between layers (readers → model → renderer → external services): + +- **Reader → Model** (extended): + ```ts + readDesignReviewLedger(projectRoot, changeName): ArtifactReadResult + readApplyReviewLedger(projectRoot, changeName): ArtifactReadResult + readDesignReviewResult(projectRoot, changeName): ArtifactReadResult + readApplyReviewResult(projectRoot, changeName): ArtifactReadResult + selectActiveReviewLedger(currentPhase): "design" | "apply" | null + ``` + All readers accept a change name (not a run id) because the artifacts live under `openspec/changes//`, consistent with `taskGraphPath`. `ReviewResultSummary` is a minimal type containing `{ summary?: string; round_index?: number }` — the watcher extracts the narrative summary and the round indicator, and ignores other fields. The `round_index` enables the model builder to validate that the summary corresponds to the ledger's latest round (see D1). + +- **Model → Renderer** (extended): the existing Review section model is replaced with a composite model carrying independent layer states: + ```ts + interface ReviewSectionModel { + // ... existing fields (round_index, max_rounds, loop_state, etc.) + readonly snapshotState: ReviewLayerState; + readonly digestState: ReviewLayerState; + } + ``` + `ReviewLayerState`, `LedgerDigest`, and `ReviewSectionModel` shapes are defined in D4. + +- **External contracts consumed**: + - `ReviewLedger` from `src/types/contracts.ts:393`. + - `LedgerRoundSummary` from `src/types/contracts.ts:374`. + - `ReviewFinding` from `src/types/contracts.ts:358`. + - No helper functions from `src/lib/review-ledger.ts` are imported (see D1/D2 for rationale). + +- **Inputs/outputs other bundles depend on**: none. The watcher is a terminal consumer; no downstream bundle reads from it. + +## Persistence / Ownership + +- **Data ownership** (unchanged): + - `review-ledger-design.json` / `review-ledger.json` are owned by review orchestration. The watcher is a read-only consumer. Spec `artifact-ownership-model` already enumerates these files. + - The watcher owns no new artifacts. +- **Storage mechanisms**: filesystem reads only; no database, no shared memory, no cache directory. +- **Artifact ownership in this change**: the delta spec at `openspec/changes//specs/realtime-progress-ui/spec.md` is owned by this change. No new artifact files are introduced in `openspec/specs/`. + +## Integration Points + +- **External systems**: none. No network, no daemon, no IPC. +- **Cross-layer dependency points**: + - Watcher depends on review-orchestration's persistence schema (`ReviewLedger`). A breaking schema change would require synchronized spec updates (tracked by `review-orchestration` and `artifact-ownership-model` specs). + - Watcher depends on `review-orchestration`'s ledger filename convention (`review-ledger-design.json` / `review-ledger.json`). Any rename must update the delta spec here. +- **Regeneration / retry / save / restore boundaries**: + - The watcher's polling fallback (≈2 s) replaces "save/restore" semantics for the digest — the next poll rebuilds the digest from disk, so malformed or partial ledger writes recover on the next successful write. + - No retry logic inside the watcher; one read attempt per redraw per source. + +## Ordering / Dependency Notes + +Foundational → dependent ordering: + +1. **C-Reader** and **C-Select** are foundational (pure functions over types). They can land in parallel, or in sequence — neither depends on the other. +2. **C-Digest-Model** depends on C-Reader and C-Select for its inputs. It should land after both. +3. **C-Digest-Render** depends on C-Digest-Model's `LedgerDigest` type. It can be developed in parallel against a stubbed model type, but integration lands after C-Digest-Model. +4. **C-Integration** depends on all of the above and lands last. + +Tests at each layer are independent: +- C-Reader and C-Select tests land with their implementations (TDD). +- C-Digest-Model tests consume small handcrafted `ReviewLedger` fixtures. +- C-Digest-Render tests consume `LedgerDigest` fixtures directly. +- C-Integration uses a tmp-dir fixture run with an on-disk ledger and asserts the rendered output. + +## Completion Conditions + +A concern is complete when: + +- **C-Reader**: `readDesignReviewLedger` / `readApplyReviewLedger` and `readDesignReviewResult` / `readApplyReviewResult` exist, unit tests cover every `ArtifactReadResult` variant (ok / absent / unreadable / malformed / empty `round_summaries` for ledger; ok / absent / unreadable / malformed / missing-summary-field / missing-round-index for review-result), decision parity validation rejects ledgers where `latest_decision` disagrees with the last round summary's decision, and `src/tests/specflow-watch-readers.test.ts` extends accordingly. `ReviewResultSummary` includes both `summary?: string` and `round_index?: number`. +- **C-Select**: `selectActiveReviewLedger` exists with exhaustive phase-value coverage including a branch for each phase listed in `selectActiveAutofixPhase`, and a parity test asserting the two selectors agree on the family mapping. +- **C-Digest-Model**: `buildDigestState(readerResult, reviewResultReaderResult, ledgerRoundCount, activeFamily)` exists returning `ReviewLayerState` and covers all observable digest states (hidden for non-review phases, absent placeholder, unreadable warning, malformed warning, empty-ledger placeholder, populated digest with all `SummaryState` variants). The composite `ReviewSectionModel` builder produces independent `snapshotState` and `digestState` sub-states with tests for: ledger-only (snapshot absent + digest ok), snapshot-only (snapshot ok + digest absent), both present, hidden (non-review phase), mixed warning/placeholder combinations (e.g., snapshot ok + digest malformed, snapshot absent + digest ok). Open-findings ranking tests verify severity → latest_round → id ordering including ties, and SHALL include test cases for `critical` severity findings ranking at the same level as `high` (not below `low`) and for unknown severity values defaulting to rank 0. Severity aggregation tests SHALL verify that `critical` findings are counted under `openSeverity.high` and that `critical` findings in `topOpen` display as `"HIGH"`. `summaryState` tests SHALL cover: `available` (round-matched summary), `stale` (round-mismatched summary with both round numbers preserved), `absent` (review-result absent or no summary field), `warning` (review-result unreadable/malformed). When `round_index` is absent from the review-result, the summary is accepted optimistically as `available`. +- **C-Digest-Render**: renderer unit tests assert the exact line sequence for each independent layer state combination (snapshot ok + digest ok, snapshot placeholder + digest ok, snapshot ok + digest warning, digest hidden for non-review phases, etc.), plus narrow-terminal (`columns < 80`) collapse/truncate behavior, plus wide-terminal full rendering. Confirms both layers render independently — a digest warning does not suppress the snapshot view and vice versa. Confirms `hidden` state produces no output for that layer. +- **C-Integration**: a full TUI test (similar to `specflow-watch-integration.test.ts`) spins up a run with a ledger on disk, triggers a redraw, and asserts the digest lines appear below the existing Review section progress view. Filesystem-watch integration: updating the ledger triggers a redraw. + +Independent reviewability: +- Readers / selector PRs can be reviewed with no UI context. +- Model PR can be reviewed against fixtures without a live TUI. +- Renderer PR can be reviewed via snapshot text fixtures. +- Integration PR is the only one that requires running the TUI. + +## Risks / Trade-offs + +- **Latest-round narrative field mismatch** → The persisted `LedgerRoundSummary` does not carry a `summary` string; free-form narrative lives in `ReviewPayload`. Mitigation: the watcher reads the `review-result-.json` artifact as a separate tolerant reader to extract both the `summary` and `round_index` fields. The model builder validates that the review-result's `round_index` matches the ledger's latest round (`round_summaries.length`). When the rounds match (or `round_index` is absent in the review-result), `summaryState` is `available`. When they disagree, `summaryState` is `stale` and the renderer annotates the line with the round mismatch. When the review-result is absent or lacks a `summary` field, `summaryState` is `absent` and the line is omitted. When the review-result is unreadable or malformed, `summaryState` is `warning` with a reason, and the renderer shows a distinct warning line — this prevents a broken summary source from being indistinguishable from a legitimately absent one. +- **Decision parity violation** → If `ledger.latest_decision` and `round_summaries[len-1].decision` are both present but disagree, the watcher treats the ledger as `malformed` and shows an inline warning. This is deliberately strict — silent inconsistency in decision data is worse than a visible warning that prompts investigation. +- **Ledger schema drift** → If `ReviewLedger` evolves (new required fields), the watcher's full-schema validator will correctly reject the old shape as `malformed`, triggering the inline warning path. This is the desired behavior — the operator sees that the ledger format has changed. Mitigation: the warning message includes the specific schema violation to aid debugging. +- **Narrow-terminal visual regression** → Truncating lines with `…` changes alignment with the existing progress rows. Mitigation: existing progress rows are already tuned for 80-column rendering; the digest truncation applies below the same threshold, so both layers collapse in step. Snapshot tests lock this in. +- **Concurrent watcher race with ledger writer** → The orchestrator writes `review-ledger.json` atomically (`rename`), but a reader observing the file mid-write on some filesystems may see a truncated snapshot. Mitigation: the reader already handles malformed → inline warning path; the next poll recovers. No lock required because the watcher is strictly read-only. +- **Multiple active ledgers** → If a run somehow has both files on disk (e.g., after re-entering design from apply), only the family-mapped one is read. Mitigation: this is the same behavior as the existing autofix-snapshot selector, so operator expectations are consistent. +- **Performance** → Each redraw reads the ledger JSON and sorts findings. For typical ledgers (<100 findings), this is sub-millisecond. Mitigation: if large ledgers appear in the wild (1000+ findings), add a per-file mtime cache to skip re-parse when unchanged — deferred unless measured regressions appear. +- **Test flakiness from fs watchers** → Adding two more watched paths slightly increases exposure to the existing polling-fallback flakiness in the filesystem-watch layer. Mitigation: the polling fallback already exists; adding paths does not change the failure mode. + +## Migration Plan + +- **Rollout**: non-breaking, additive. New behavior appears the next time an operator runs `specflow-watch` on a run that has a ledger on disk. +- **Compatibility**: runs that predate the change have no ledger files — the watcher renders `No review digest yet` and all other sections unchanged. +- **Rollback**: revert the PR; the watcher falls back to pre-change rendering with no data loss (nothing persists). +- **No migrations required**: no schema changes, no data migrations, no config changes. + +## Open Questions + +1. ~~**Latest summary source**~~ — **Resolved**: the watcher reads `review-result-design.json` / `review-result.json` (family-mapped) as a separate tolerant reader to extract the `summary` and `round_index` fields. The model validates `round_index` against the ledger's latest round to prevent stale-narrative pairing. The `summaryState` discriminated union (D1/D4) distinguishes `available`, `stale`, `absent`, and `warning` — each with distinct renderer behavior. This avoids deferring the acceptance criterion to a Phase 2 and ensures broken/stale/absent summary sources are never conflated. +2. ~~**`critical` severity handling**~~ — **Resolved**: `critical` findings map to the same rank as `high` in D6 (rank 0), are aggregated into the `high` count in the severity breakdown (D1), and render as `HIGH` in the top-3 findings display (D4). Unknown severities also default to rank 0 and aggregate into `high`. This ensures `critical` findings are never understated, hidden from the top-3 list, or ranked below `low`. The digest retains the three-tier `HIGH | MEDIUM | LOW` display format — a separate `CRITICAL` display tier can be added in a future change if needed. +3. **Title truncation cap** — the top-3 findings display finding titles unbounded. On 80–120 column terminals this is fine; on wider terminals it is fine; on <80 columns the entire block is collapsed. Is an explicit per-row title cap (e.g., 60 chars) worth adding even on wide terminals for consistency? (Preference: no cap in Phase 1; revisit if long titles appear.) diff --git a/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/proposal.md b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/proposal.md new file mode 100644 index 0000000..5799b17 --- /dev/null +++ b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/proposal.md @@ -0,0 +1,63 @@ +## Why + +Source: [issue #183](https://github.com/skr19930617/specflow/issues/183) — `Extend specflow-watch with review ledger digests and subagent activity`. + +The `specflow-watch` TUI currently resolves the active review family from `current_phase` and renders round/progress counters sourced from `autofix-progress-design_review.json` / `autofix-progress-apply_review.json`. However, it never reads `review-ledger-design.json` or `review-ledger.json`, so the operator can see *what round is running* but not *what the review concluded* — no decision, no finding totals, no severity distribution, no digest of active findings. + +The review data contracts already persist rich result state (`ReviewResult`, `ReviewLedger`, `LedgerSnapshot`, `ReviewFinding`), and `specflow-watch` is already structured as tolerant artifact readers → pure model builder → pure renderer. Adding a ledger-backed digest is a natural, additive widening of the read model that meaningfully improves operator visibility without changing review-orchestration contracts, ledger persistence, or the write boundary (the watcher remains strictly read-only). + +## What Changes + +- **Extend artifact readers** — add tolerant readers (`ok` / `absent` / `unreadable` / `malformed`) for `openspec/changes//review-ledger-design.json` and `openspec/changes//review-ledger.json`, matching the existing tolerant-reader style. +- **Select ledger by review family** — reuse the existing family rule from the snapshot selector: `design_draft | design_review | design_ready` → design ledger; `apply_draft | apply_review | apply_ready | approved` → apply ledger; any other phase → no active ledger digest. +- **Preserve the existing progress layer** — snapshot-based round/loop_state/live/completed/manual-fix display stays unchanged. +- **Add a ledger digest layer** to the existing Review section, rendering (in this order): + 1. latest decision (e.g. `approve_with_findings`) + 2. counts `total / open / new / resolved` + 3. severity breakdown `HIGH n | MEDIUM n | LOW n` + 4. latest round summary text + 5. top 3 unresolved findings (severity + short title) +- **Degrade gracefully per source** — `snapshot-only` and `ledger-only` cases each still render their half. Run-state remains the only mandatory source. Specifically: + - **Ledger absent** → single-line placeholder `No review digest yet` in the Review section, below the existing progress view. + - **Ledger unreadable (I/O failure)** → inline warning line `Review ledger unreadable: ` scoped to the Review section; other sections render normally. + - **Ledger malformed (parse failure)** → inline warning line in the Review section; other sections render normally. + - **Ledger present but zero `LedgerSnapshot` entries** → same `No review digest yet` placeholder as the absent case (operator-facing state is identical: no review outcome yet). +- **Keep compact** — render a *digest*, not full ledger content. The TUI does not become a ledger browser. No write paths, no contract changes, no observation-event changes. + +**Clarified decisions:** + +From the issue's own open questions: +- **Digest source** — latest round only. The digest mirrors the latest `LedgerSnapshot`'s fields; no cross-round aggregation. +- **Unresolved-findings ranking** — severity (`HIGH` > `MEDIUM` > `LOW`), then recency within the same severity. Top 3 open findings only. +- **Narrow-terminal behavior** — below an 80-column threshold, auto-collapse the top-3 open-findings list; retain the decision / counts / severity / summary lines. + +From challenge-reclarify: +- **Severity breakdown set (C3)** — counts **open findings only** (status `open` / `new`), not resolved or overridden findings. The `Findings` line already carries `total | open | new | resolved`, so the severity breakdown focuses on what remains actionable. +- **Recency tie-break (C4)** — ranking within a severity uses `latest_round` DESC; ties broken by finding `id` ASC (e.g. `R3-F02` before `R3-F05`). `ReviewFinding` has no wall-clock timestamps, so `latest_round` is the authoritative recency signal. +- **Empty ledger handling (C5)** — a ledger file that parses but has zero `LedgerSnapshot` entries renders the same `No review digest yet` placeholder as the absent-file case. +- **Narrow-terminal overflow (C6)** — exact threshold is **80 columns**. Below 80: findings list is hidden, and the remaining decision / counts / severity / summary lines are truncated with an ellipsis (`…`) instead of wrapping, preserving alignment with existing progress rows. + +**Title alignment note**: the issue title mentions "subagent activity" but the body focuses entirely on review ledger digests. This change scopes to the ledger digest only; any subagent-activity work is out of scope and would be a separate proposal. + +## Capabilities + +### New Capabilities +- None. + +### Modified Capabilities +- `realtime-progress-ui`: add the ledger digest layer (new requirement), extend the read-only artifact contract to include the two review-ledger files, and extend graceful-degradation rules to cover ledger-present / ledger-absent / ledger-malformed combinations alongside the existing snapshot cases. The existing snapshot-based progress requirement and family selection rules stay intact. + +## Impact + +- **Code** + - `src/lib/specflow-watch/artifact-readers.ts` — add `readDesignReviewLedger` / `readApplyReviewLedger` with tolerant status envelope. + - watch model builder — extend the review section model with optional `ledgerDigest` fields (`decision`, `counts`, `severity`, `latestSummary`, `topUnresolved`). + - watch renderer — render digest lines below existing progress lines; handle `No review digest yet` placeholder and malformed-ledger warnings. + - `src/bin/specflow-watch.ts` (TUI entrypoint) — wire new readers → model → renderer path. + - new unit + snapshot tests for ledger reader tolerance, digest rendering across family states, and the ledger-present/absent/malformed matrix. +- **Specs** + - `openspec/changes//specs/realtime-progress-ui/spec.md` — delta adding digest requirement + extended degradation scenarios. +- **Contracts & dependencies** + - Consumes existing `ReviewLedger` / `LedgerSnapshot` shapes from `src/lib/review-ledger.ts`; no schema changes. + - No changes to `workflow-observation-events`, `run-artifact-store-conformance`, or `review-autofix-progress-observability`. +- **Out of scope** — subagent activity surfacing, write paths, full ledger browser UI, raw ledger JSON rendering, changes to review orchestration. diff --git a/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/review-ledger-design.json b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/review-ledger-design.json new file mode 100644 index 0000000..9300aeb --- /dev/null +++ b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/review-ledger-design.json @@ -0,0 +1,190 @@ +{ + "feature_id": "extend-specflow-watch-with-review-ledger-digests-and-subagent-activity", + "phase": "design", + "current_round": 5, + "status": "has_open_high", + "max_finding_id": 8, + "findings": [ + { + "id": "R1-F01", + "severity": "high", + "category": "completeness", + "title": "Latest summary requirement has no compliant data source", + "detail": "The spec requires a `Latest summary:` line sourced from the latest review outcome, but the design itself notes that persisted `ReviewLedger` and `LedgerRoundSummary` data do not contain narrative summary text. D1 then proposes substituting `decision` or omitting the line and deferring a real source to Phase 2, and task 1.3 only 'freezes' that fallback. That leaves an acceptance criterion unmet. Resolve this before implementation by either changing the allowed artifact contract/spec to include a read-only summary source, or extending the persisted ledger contract so the latest summary text is actually available.", + "origin_round": 1, + "latest_round": 1, + "status": "open", + "relation": "new", + "supersedes": null, + "notes": "" + }, + { + "id": "R1-F02", + "severity": "medium", + "category": "consistency", + "title": "Review section model does not define independent progress and digest states", + "detail": "The spec requires snapshot-only and ledger-only modes to each render their own half of the Review section. The design adds `ledgerDigest` to an abstract review model, but does not define a composite state model for independent snapshot placeholder/warning state versus digest placeholder/warning state. In the current watcher architecture, the review section is a single section-state, so this omission risks suppressing the digest whenever snapshot progress is absent or malformed. Define an explicit composite review-section interface and add tests for ledger-only, snapshot-only, and mixed warning/placeholder combinations.", + "origin_round": 1, + "latest_round": 1, + "status": "resolved", + "relation": "new", + "supersedes": null, + "notes": "", + "resolved_round": 2 + }, + { + "id": "R1-F03", + "severity": "medium", + "category": "consistency", + "title": "Loose ledger validation conflicts with the malformed-ledger contract", + "detail": "D2 intentionally validates only the subset of ledger fields used by the digest, while the spec says malformed handling applies when the ledger does not conform to the review-ledger schema. With the proposed validator, schema-invalid ledgers could still render as usable, hiding drift and skipping the required inline warning path. Either validate against the full read-only `ReviewLedger` contract or explicitly change the spec; task 2.2 should be updated accordingly.", + "origin_round": 1, + "latest_round": 1, + "status": "resolved", + "relation": "new", + "supersedes": null, + "notes": "", + "resolved_round": 2 + }, + { + "id": "R2-F04", + "severity": "medium", + "category": "completeness", + "file": "openspec/changes/extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/design.md", + "title": "Digest state cannot represent the required non-review suppression case", + "detail": "The spec requires that non-review phases do not read or render any ledger digest at all, but D4's ReviewLayerState only allows ok, placeholder, or warning and the renderer is defined to emit one of those states in sequence. That leaves no explicit way to distinguish 'active family with no digest yet' from 'phase is outside the review families', so the current model pushes the implementation toward showing `No review digest yet` on phases like `spec_draft` or `proposal_clarify`. Add an explicit hidden/not_applicable digest state (or make digestState optional) and add model/render tests for non-review phases.", + "origin_round": 2, + "latest_round": 2, + "status": "resolved", + "relation": "new", + "supersedes": null, + "notes": "", + "resolved_round": 3 + }, + { + "id": "R2-F05", + "severity": "medium", + "category": "consistency", + "file": "openspec/changes/extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/design.md", + "title": "Decision source is ambiguous between the top-level ledger field and the latest round", + "detail": "D1 says the digest should prefer `ledger.latest_decision` and only fall back to the last `round_summaries` entry, while task 3.3 describes building the digest from the last round summary plus open findings and the spec defines the digest as latest-round/latest-snapshot data. If those fields ever diverge, the decision line can disagree with the counts and open-finding data taken from the last round. Pick a single source of truth for the decision line, or validate parity and treat mismatches as malformed, then align the tasks and tests to that rule.", + "origin_round": 2, + "latest_round": 2, + "status": "resolved", + "relation": "new", + "supersedes": null, + "notes": "", + "resolved_round": 3 + }, + { + "id": "R3-F06", + "severity": "high", + "category": "consistency", + "file": "openspec/changes/extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/design.md", + "title": "Review-result summary handling does not guarantee a correct latest-round line", + "detail": "D1 and tasks 1.3/2.3/3.2 make `Latest summary:` depend on `review-result-.json`, but they only extract `{ summary?: string }` and do not define how that artifact is proven to match `round_summaries[last]`. That leaves the digest free to pair a newer ledger decision/counts block with a stale narrative summary from an older round. The same path also collapses unreadable/malformed review-result files into `latestSummary = null`, making a broken summary source indistinguishable from a legitimately absent summary. Either move the summary into the ledger-backed contract, or extend the summary-source contract so it can be validated against the latest round and can surface warning states separately from 'summary absent'.", + "origin_round": 3, + "latest_round": 3, + "status": "open", + "relation": "new", + "supersedes": null, + "notes": "" + }, + { + "id": "R4-F07", + "severity": "medium", + "category": "consistency", + "file": "openspec/changes/extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/design.md", + "title": "Critical severity handling is left undefined even though current ledgers use it", + "detail": "`ReviewFinding.severity` already allows real `critical` values and the existing review-orchestration contract treats `critical` and `high` as the blocking severities. D6 currently ranks unknown severities below `low`, and the digest contract only renders HIGH/MEDIUM/LOW counts. On a valid ledger with open critical findings, this design can understate the most severe open work and hide critical findings from the top-3 list. Define the Phase 1 mapping explicitly before implementation, either by extending the spec to surface CRITICAL or by mapping `critical` to HIGH consistently in ranking, counts, and rendering, then add tests for that behavior.", + "origin_round": 4, + "latest_round": 4, + "status": "resolved", + "relation": "new", + "supersedes": null, + "notes": "", + "resolved_round": 5 + }, + { + "id": "R5-F08", + "severity": "medium", + "category": "completeness", + "file": "openspec/changes/extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/design.md", + "title": "Zero-open digest behavior is not specified tightly enough to satisfy the spec", + "detail": "The spec requires the `Open findings:` header and rows to be omitted entirely when the latest digest has zero open findings. D4 only says an `ok` digest renders the top-3 block, and task 3.7 covers populated digests and empty ledgers but not the `topOpen = []` case for a non-empty ledger. That leaves room for an implementation that still prints an empty `Open findings:` header. Add an explicit render rule and a test for a readable ledger whose latest state has zero open findings.", + "origin_round": 5, + "latest_round": 5, + "status": "new", + "relation": "new", + "supersedes": null, + "notes": "" + } + ], + "round_summaries": [ + { + "round": 1, + "total": 3, + "open": 3, + "new": 3, + "resolved": 0, + "overridden": 0, + "by_severity": { + "high": 1, + "medium": 2 + }, + "gate_id": "review_decision-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity-1-design_review-1" + }, + { + "round": 2, + "total": 5, + "open": 3, + "new": 2, + "resolved": 2, + "overridden": 0, + "by_severity": { + "high": 1, + "medium": 2 + }, + "gate_id": "review_decision-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity-1-design_review-2" + }, + { + "round": 3, + "total": 6, + "open": 2, + "new": 1, + "resolved": 4, + "overridden": 0, + "by_severity": { + "high": 1, + "medium": 1 + }, + "gate_id": "review_decision-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity-1-design_review-3" + }, + { + "round": 4, + "total": 7, + "open": 3, + "new": 1, + "resolved": 4, + "overridden": 0, + "by_severity": { + "high": 2, + "medium": 1 + }, + "gate_id": "review_decision-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity-1-design_review-4" + }, + { + "round": 5, + "total": 8, + "open": 3, + "new": 1, + "resolved": 5, + "overridden": 0, + "by_severity": { + "high": 2, + "medium": 1 + }, + "gate_id": "review_decision-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity-1-design_review-5" + } + ] +} diff --git a/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/review-ledger-design.json.bak b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/review-ledger-design.json.bak new file mode 100644 index 0000000..6bd4740 --- /dev/null +++ b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/review-ledger-design.json.bak @@ -0,0 +1,189 @@ +{ + "feature_id": "extend-specflow-watch-with-review-ledger-digests-and-subagent-activity", + "phase": "design", + "current_round": 5, + "status": "has_open_high", + "max_finding_id": 8, + "findings": [ + { + "id": "R1-F01", + "severity": "high", + "category": "completeness", + "title": "Latest summary requirement has no compliant data source", + "detail": "The spec requires a `Latest summary:` line sourced from the latest review outcome, but the design itself notes that persisted `ReviewLedger` and `LedgerRoundSummary` data do not contain narrative summary text. D1 then proposes substituting `decision` or omitting the line and deferring a real source to Phase 2, and task 1.3 only 'freezes' that fallback. That leaves an acceptance criterion unmet. Resolve this before implementation by either changing the allowed artifact contract/spec to include a read-only summary source, or extending the persisted ledger contract so the latest summary text is actually available.", + "origin_round": 1, + "latest_round": 1, + "status": "open", + "relation": "new", + "supersedes": null, + "notes": "" + }, + { + "id": "R1-F02", + "severity": "medium", + "category": "consistency", + "title": "Review section model does not define independent progress and digest states", + "detail": "The spec requires snapshot-only and ledger-only modes to each render their own half of the Review section. The design adds `ledgerDigest` to an abstract review model, but does not define a composite state model for independent snapshot placeholder/warning state versus digest placeholder/warning state. In the current watcher architecture, the review section is a single section-state, so this omission risks suppressing the digest whenever snapshot progress is absent or malformed. Define an explicit composite review-section interface and add tests for ledger-only, snapshot-only, and mixed warning/placeholder combinations.", + "origin_round": 1, + "latest_round": 1, + "status": "resolved", + "relation": "new", + "supersedes": null, + "notes": "", + "resolved_round": 2 + }, + { + "id": "R1-F03", + "severity": "medium", + "category": "consistency", + "title": "Loose ledger validation conflicts with the malformed-ledger contract", + "detail": "D2 intentionally validates only the subset of ledger fields used by the digest, while the spec says malformed handling applies when the ledger does not conform to the review-ledger schema. With the proposed validator, schema-invalid ledgers could still render as usable, hiding drift and skipping the required inline warning path. Either validate against the full read-only `ReviewLedger` contract or explicitly change the spec; task 2.2 should be updated accordingly.", + "origin_round": 1, + "latest_round": 1, + "status": "resolved", + "relation": "new", + "supersedes": null, + "notes": "", + "resolved_round": 2 + }, + { + "id": "R2-F04", + "severity": "medium", + "category": "completeness", + "file": "openspec/changes/extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/design.md", + "title": "Digest state cannot represent the required non-review suppression case", + "detail": "The spec requires that non-review phases do not read or render any ledger digest at all, but D4's ReviewLayerState only allows ok, placeholder, or warning and the renderer is defined to emit one of those states in sequence. That leaves no explicit way to distinguish 'active family with no digest yet' from 'phase is outside the review families', so the current model pushes the implementation toward showing `No review digest yet` on phases like `spec_draft` or `proposal_clarify`. Add an explicit hidden/not_applicable digest state (or make digestState optional) and add model/render tests for non-review phases.", + "origin_round": 2, + "latest_round": 2, + "status": "resolved", + "relation": "new", + "supersedes": null, + "notes": "", + "resolved_round": 3 + }, + { + "id": "R2-F05", + "severity": "medium", + "category": "consistency", + "file": "openspec/changes/extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/design.md", + "title": "Decision source is ambiguous between the top-level ledger field and the latest round", + "detail": "D1 says the digest should prefer `ledger.latest_decision` and only fall back to the last `round_summaries` entry, while task 3.3 describes building the digest from the last round summary plus open findings and the spec defines the digest as latest-round/latest-snapshot data. If those fields ever diverge, the decision line can disagree with the counts and open-finding data taken from the last round. Pick a single source of truth for the decision line, or validate parity and treat mismatches as malformed, then align the tasks and tests to that rule.", + "origin_round": 2, + "latest_round": 2, + "status": "resolved", + "relation": "new", + "supersedes": null, + "notes": "", + "resolved_round": 3 + }, + { + "id": "R3-F06", + "severity": "high", + "category": "consistency", + "file": "openspec/changes/extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/design.md", + "title": "Review-result summary handling does not guarantee a correct latest-round line", + "detail": "D1 and tasks 1.3/2.3/3.2 make `Latest summary:` depend on `review-result-.json`, but they only extract `{ summary?: string }` and do not define how that artifact is proven to match `round_summaries[last]`. That leaves the digest free to pair a newer ledger decision/counts block with a stale narrative summary from an older round. The same path also collapses unreadable/malformed review-result files into `latestSummary = null`, making a broken summary source indistinguishable from a legitimately absent summary. Either move the summary into the ledger-backed contract, or extend the summary-source contract so it can be validated against the latest round and can surface warning states separately from 'summary absent'.", + "origin_round": 3, + "latest_round": 3, + "status": "open", + "relation": "new", + "supersedes": null, + "notes": "" + }, + { + "id": "R4-F07", + "severity": "medium", + "category": "consistency", + "file": "openspec/changes/extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/design.md", + "title": "Critical severity handling is left undefined even though current ledgers use it", + "detail": "`ReviewFinding.severity` already allows real `critical` values and the existing review-orchestration contract treats `critical` and `high` as the blocking severities. D6 currently ranks unknown severities below `low`, and the digest contract only renders HIGH/MEDIUM/LOW counts. On a valid ledger with open critical findings, this design can understate the most severe open work and hide critical findings from the top-3 list. Define the Phase 1 mapping explicitly before implementation, either by extending the spec to surface CRITICAL or by mapping `critical` to HIGH consistently in ranking, counts, and rendering, then add tests for that behavior.", + "origin_round": 4, + "latest_round": 4, + "status": "resolved", + "relation": "new", + "supersedes": null, + "notes": "", + "resolved_round": 5 + }, + { + "id": "R5-F08", + "severity": "medium", + "category": "completeness", + "file": "openspec/changes/extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/design.md", + "title": "Zero-open digest behavior is not specified tightly enough to satisfy the spec", + "detail": "The spec requires the `Open findings:` header and rows to be omitted entirely when the latest digest has zero open findings. D4 only says an `ok` digest renders the top-3 block, and task 3.7 covers populated digests and empty ledgers but not the `topOpen = []` case for a non-empty ledger. That leaves room for an implementation that still prints an empty `Open findings:` header. Add an explicit render rule and a test for a readable ledger whose latest state has zero open findings.", + "origin_round": 5, + "latest_round": 5, + "status": "new", + "relation": "new", + "supersedes": null, + "notes": "" + } + ], + "round_summaries": [ + { + "round": 1, + "total": 3, + "open": 3, + "new": 3, + "resolved": 0, + "overridden": 0, + "by_severity": { + "high": 1, + "medium": 2 + }, + "gate_id": "review_decision-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity-1-design_review-1" + }, + { + "round": 2, + "total": 5, + "open": 3, + "new": 2, + "resolved": 2, + "overridden": 0, + "by_severity": { + "high": 1, + "medium": 2 + }, + "gate_id": "review_decision-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity-1-design_review-2" + }, + { + "round": 3, + "total": 6, + "open": 2, + "new": 1, + "resolved": 4, + "overridden": 0, + "by_severity": { + "high": 1, + "medium": 1 + }, + "gate_id": "review_decision-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity-1-design_review-3" + }, + { + "round": 4, + "total": 7, + "open": 3, + "new": 1, + "resolved": 4, + "overridden": 0, + "by_severity": { + "high": 2, + "medium": 1 + }, + "gate_id": "review_decision-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity-1-design_review-4" + }, + { + "round": 5, + "total": 8, + "open": 3, + "new": 1, + "resolved": 5, + "overridden": 0, + "by_severity": { + "high": 2, + "medium": 1 + } + } + ] +} diff --git a/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/review-ledger.json b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/review-ledger.json new file mode 100644 index 0000000..31b8156 --- /dev/null +++ b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/review-ledger.json @@ -0,0 +1,119 @@ +{ + "feature_id": "extend-specflow-watch-with-review-ledger-digests-and-subagent-activity", + "phase": "impl", + "current_round": 3, + "status": "has_open_high", + "max_finding_id": 3, + "findings": [ + { + "id": "R3-F01", + "severity": "high", + "category": "completeness", + "file": "src/lib/watch-renderer/model.ts", + "title": "Digest never renders the required latest-summary line", + "detail": "`buildSummaryState()` is hard-coded to return `{ kind: \"absent\" }`, and the watcher adds no family-mapped `readDesignReviewResult` / `readApplyReviewResult` reader or summary-state wiring. As a result, `Latest summary:` can never appear, even when the current review has narrative summary text. The spec/design require that line when summary text is available, plus distinct `stale` and `warning` handling for mismatched or unreadable summary sources. Implement the summary-source reader path and plumb `available` / `stale` / `absent` / `warning` into the digest renderer instead of treating summary as permanently unavailable.", + "origin_round": 3, + "latest_round": 3, + "status": "new", + "relation": "new", + "supersedes": null, + "notes": "" + }, + { + "id": "R3-F02", + "severity": "medium", + "category": "testing", + "file": "src/tests/watch-renderer.test.ts", + "title": "New tests lock in the spec-violating summary omission", + "detail": "The added tests explicitly assert that the digest omits `Latest summary:` because \"no compliant persisted summary source exists today.\" That guards behavior the spec/design do not allow, and there is still no coverage for the required summary-source matrix (`available`, `stale`, `absent`, `warning`) or the missing review-result readers. Replace these assertions with tests for the actual summary contract once the feature is implemented.", + "origin_round": 3, + "latest_round": 3, + "status": "new", + "relation": "new", + "supersedes": null, + "notes": "" + }, + { + "id": "R1-F01", + "severity": "high", + "category": "correctness", + "file": "src/lib/watch-renderer/model.ts", + "title": "Digest summary is sourced from a second artifact instead of the ledger", + "detail": "The proposal makes the digest ledger-backed, says it should mirror the latest round data, and explicitly requires ledger-only mode to still render the digest. This implementation only shows the summary line when `review-result-design.json` / `review-result.json` is present, so a valid ledger by itself cannot satisfy acceptance criterion 4 (`latest round summary text`). It also widens the read/watch surface beyond the two ledger files called out by the spec. Populate the summary from the latest ledger snapshot/round entry instead of depending on `review-result*.json`.", + "origin_round": 1, + "latest_round": 2, + "status": "resolved", + "relation": "same", + "supersedes": null, + "notes": "" + }, + { + "id": "R1-F02", + "severity": "medium", + "category": "correctness", + "file": "src/lib/watch-renderer/model.ts", + "title": "Staleness is compared against array length instead of the actual latest round number", + "detail": "`buildDigestFromLedger()` passes `round_summaries.length` into `buildSummaryState()`. That is not the same thing as the latest round identifier. If round numbers ever skip, are compacted, or stop matching array length, a current summary will be marked stale (or a stale one will be treated as current). Compare `round_index` against `last.round` or `ledger.current_round`, and add a regression test where the latest round number differs from the array length.", + "origin_round": 1, + "latest_round": 1, + "status": "resolved", + "relation": "new", + "supersedes": null, + "notes": "" + }, + { + "id": "R1-F03", + "severity": "medium", + "category": "error_handling", + "file": "src/lib/specflow-watch/artifact-readers.ts", + "title": "Malformed findings can slip through schema validation as `ok`", + "detail": "`validateReviewLedgerSchema()` is intended to reject malformed ledgers, but `validateReviewFinding()` only type-checks fields if they exist. Objects like `{}` or `{ \"status\": \"open\" }` inside `findings[]` currently pass validation, which means the watcher can render blank titles/IDs or silently drop bad data instead of showing the required malformed-ledger warning. Require the fields the digest depends on (`id`, `title`, `severity`, `status`, and any contract-mandated fields) and cover those cases in tests.", + "origin_round": 1, + "latest_round": 1, + "status": "resolved", + "relation": "new", + "supersedes": null, + "notes": "" + } + ], + "round_summaries": [ + { + "round": 1, + "total": 3, + "open": 3, + "new": 3, + "resolved": 0, + "overridden": 0, + "by_severity": { + "high": 1, + "medium": 2 + }, + "gate_id": "review_decision-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity-1-apply_review-1" + }, + { + "round": 2, + "total": 3, + "open": 1, + "new": 0, + "resolved": 2, + "overridden": 0, + "by_severity": { + "high": 1 + }, + "gate_id": "review_decision-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity-1-apply_review-2" + }, + { + "round": 3, + "total": 5, + "open": 2, + "new": 2, + "resolved": 3, + "overridden": 0, + "by_severity": { + "high": 1, + "medium": 1 + }, + "gate_id": "review_decision-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity-1-apply_review-3" + } + ] +} diff --git a/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/review-ledger.json.bak b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/review-ledger.json.bak new file mode 100644 index 0000000..4c83914 --- /dev/null +++ b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/review-ledger.json.bak @@ -0,0 +1,118 @@ +{ + "feature_id": "extend-specflow-watch-with-review-ledger-digests-and-subagent-activity", + "phase": "impl", + "current_round": 3, + "status": "has_open_high", + "max_finding_id": 3, + "findings": [ + { + "id": "R3-F01", + "severity": "high", + "category": "completeness", + "file": "src/lib/watch-renderer/model.ts", + "title": "Digest never renders the required latest-summary line", + "detail": "`buildSummaryState()` is hard-coded to return `{ kind: \"absent\" }`, and the watcher adds no family-mapped `readDesignReviewResult` / `readApplyReviewResult` reader or summary-state wiring. As a result, `Latest summary:` can never appear, even when the current review has narrative summary text. The spec/design require that line when summary text is available, plus distinct `stale` and `warning` handling for mismatched or unreadable summary sources. Implement the summary-source reader path and plumb `available` / `stale` / `absent` / `warning` into the digest renderer instead of treating summary as permanently unavailable.", + "origin_round": 3, + "latest_round": 3, + "status": "new", + "relation": "new", + "supersedes": null, + "notes": "" + }, + { + "id": "R3-F02", + "severity": "medium", + "category": "testing", + "file": "src/tests/watch-renderer.test.ts", + "title": "New tests lock in the spec-violating summary omission", + "detail": "The added tests explicitly assert that the digest omits `Latest summary:` because \"no compliant persisted summary source exists today.\" That guards behavior the spec/design do not allow, and there is still no coverage for the required summary-source matrix (`available`, `stale`, `absent`, `warning`) or the missing review-result readers. Replace these assertions with tests for the actual summary contract once the feature is implemented.", + "origin_round": 3, + "latest_round": 3, + "status": "new", + "relation": "new", + "supersedes": null, + "notes": "" + }, + { + "id": "R1-F01", + "severity": "high", + "category": "correctness", + "file": "src/lib/watch-renderer/model.ts", + "title": "Digest summary is sourced from a second artifact instead of the ledger", + "detail": "The proposal makes the digest ledger-backed, says it should mirror the latest round data, and explicitly requires ledger-only mode to still render the digest. This implementation only shows the summary line when `review-result-design.json` / `review-result.json` is present, so a valid ledger by itself cannot satisfy acceptance criterion 4 (`latest round summary text`). It also widens the read/watch surface beyond the two ledger files called out by the spec. Populate the summary from the latest ledger snapshot/round entry instead of depending on `review-result*.json`.", + "origin_round": 1, + "latest_round": 2, + "status": "resolved", + "relation": "same", + "supersedes": null, + "notes": "" + }, + { + "id": "R1-F02", + "severity": "medium", + "category": "correctness", + "file": "src/lib/watch-renderer/model.ts", + "title": "Staleness is compared against array length instead of the actual latest round number", + "detail": "`buildDigestFromLedger()` passes `round_summaries.length` into `buildSummaryState()`. That is not the same thing as the latest round identifier. If round numbers ever skip, are compacted, or stop matching array length, a current summary will be marked stale (or a stale one will be treated as current). Compare `round_index` against `last.round` or `ledger.current_round`, and add a regression test where the latest round number differs from the array length.", + "origin_round": 1, + "latest_round": 1, + "status": "resolved", + "relation": "new", + "supersedes": null, + "notes": "" + }, + { + "id": "R1-F03", + "severity": "medium", + "category": "error_handling", + "file": "src/lib/specflow-watch/artifact-readers.ts", + "title": "Malformed findings can slip through schema validation as `ok`", + "detail": "`validateReviewLedgerSchema()` is intended to reject malformed ledgers, but `validateReviewFinding()` only type-checks fields if they exist. Objects like `{}` or `{ \"status\": \"open\" }` inside `findings[]` currently pass validation, which means the watcher can render blank titles/IDs or silently drop bad data instead of showing the required malformed-ledger warning. Require the fields the digest depends on (`id`, `title`, `severity`, `status`, and any contract-mandated fields) and cover those cases in tests.", + "origin_round": 1, + "latest_round": 1, + "status": "resolved", + "relation": "new", + "supersedes": null, + "notes": "" + } + ], + "round_summaries": [ + { + "round": 1, + "total": 3, + "open": 3, + "new": 3, + "resolved": 0, + "overridden": 0, + "by_severity": { + "high": 1, + "medium": 2 + }, + "gate_id": "review_decision-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity-1-apply_review-1" + }, + { + "round": 2, + "total": 3, + "open": 1, + "new": 0, + "resolved": 2, + "overridden": 0, + "by_severity": { + "high": 1 + }, + "gate_id": "review_decision-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity-1-apply_review-2" + }, + { + "round": 3, + "total": 5, + "open": 2, + "new": 2, + "resolved": 3, + "overridden": 0, + "by_severity": { + "high": 1, + "medium": 1 + } + } + ] +} diff --git a/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/specs/realtime-progress-ui/spec.md b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/specs/realtime-progress-ui/spec.md new file mode 100644 index 0000000..16e8385 --- /dev/null +++ b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/specs/realtime-progress-ui/spec.md @@ -0,0 +1,197 @@ +## ADDED Requirements + +### Requirement: Review section renders a ledger digest below snapshot progress + +The Review section of the TUI SHALL render a **ledger digest layer** below the existing snapshot-based progress lines whenever a review-ledger file matching the active review family is available. The digest SHALL NOT replace the existing snapshot-derived progress view; both layers coexist. + +Family-to-ledger mapping SHALL match the snapshot family rule already defined for the Review round section: + +- `current_phase ∈ {design_draft, design_review, design_ready}` → read `openspec/changes//review-ledger-design.json`. +- `current_phase ∈ {apply_draft, apply_review, apply_ready, approved}` → read `openspec/changes//review-ledger.json`. +- Any other phase → no ledger digest is rendered for this run. + +When a ledger is readable, the digest SHALL be built from the **latest `LedgerSnapshot`** only (no cross-round aggregation), and SHALL contain, in this order: + +1. `Decision: ` — the latest snapshot's decision verbatim (e.g. `approve_with_findings`). If the snapshot has no decision value, the renderer SHALL display `Decision: (none)`. +2. `Findings: total | open | new | resolved` — counts derived from the latest snapshot. +3. `Severity: HIGH | MEDIUM | LOW ` — computed **over open findings only** (`status ∈ {open, new}` in the latest snapshot). Resolved and overridden findings SHALL NOT contribute to this breakdown. +4. `Latest summary: ` — the latest round-summary text from the snapshot. If absent, the line SHALL be omitted. +5. `Open findings:` — a header line followed by up to three rows listing the top unresolved findings as ` `. Exactly three findings SHALL be shown when at least three are open; fewer rows are shown when fewer open findings exist. The header line and its rows SHALL be omitted entirely when zero open findings exist. + +Ranking of the open findings list SHALL be: + +- Primary sort: severity, in the order `HIGH` > `MEDIUM` > `LOW`. +- Secondary sort (same severity): `latest_round` DESC — findings touched in the most recent round first. +- Tertiary sort (same severity and same `latest_round`): finding `id` ASC, interpreted as a stable string comparison. + +The digest SHALL remain visible in the adjacent completed-family phases (`design_ready`, `apply_ready`, `approved`) consistent with the existing family-based review-section visibility rule. + +#### Scenario: Digest renders under snapshot progress during apply_review + +- **WHEN** `current_phase == "apply_review"`, an `autofix-progress-apply_review.json` snapshot exists, and `review-ledger.json` contains at least one `LedgerSnapshot` +- **THEN** the Review section SHALL render the existing snapshot progress lines unchanged +- **AND** below those lines it SHALL render a `Decision: …` line, a `Findings: … total | … open | … new | … resolved` line, a `Severity: HIGH … | MEDIUM … | LOW …` line, a `Latest summary: …` line, and an `Open findings:` block drawn from the **latest** `LedgerSnapshot` + +#### Scenario: Severity breakdown only counts open findings + +- **WHEN** the latest snapshot contains findings with mixed statuses (for example, 1 resolved HIGH, 2 open HIGH, 1 open MEDIUM, 1 overridden LOW) +- **THEN** the `Severity:` line SHALL display `HIGH 2 | MEDIUM 1 | LOW 0` +- **AND** resolved and overridden findings SHALL NOT contribute to the severity counts + +#### Scenario: Open findings list ranks by severity then latest_round then id + +- **WHEN** the latest snapshot contains five open findings with the following `(severity, latest_round, id)` tuples: `(MEDIUM, 4, R4-F01)`, `(HIGH, 2, R2-F03)`, `(HIGH, 3, R3-F01)`, `(LOW, 5, R5-F02)`, `(HIGH, 3, R3-F02)` +- **THEN** the `Open findings:` block SHALL render exactly three rows +- **AND** the rows SHALL be, in order: the `HIGH` at `latest_round=3, id=R3-F01`, the `HIGH` at `latest_round=3, id=R3-F02`, then the `HIGH` at `latest_round=2, id=R2-F03` +- **AND** the `MEDIUM` and `LOW` findings SHALL NOT appear in the block + +#### Scenario: Digest remains visible on apply_ready after review completes + +- **WHEN** `current_phase == "apply_ready"` and `review-ledger.json` contains a terminal `LedgerSnapshot` +- **THEN** the Review section SHALL render the digest below the completed snapshot progress view +- **AND** the digest SHALL reflect the latest `LedgerSnapshot` + +#### Scenario: Digest is suppressed on non-review phases + +- **WHEN** `current_phase` is `spec_draft`, `proposal_clarify`, or any other non-review-family phase +- **THEN** the Review section SHALL NOT read or render a ledger digest regardless of whether `review-ledger.json` or `review-ledger-design.json` exists on disk + +#### Scenario: Latest round summary missing elides the line + +- **WHEN** the latest snapshot has no round-summary text +- **THEN** the renderer SHALL omit the `Latest summary:` line entirely +- **AND** the other digest lines SHALL still render + +### Requirement: Digest degrades compactly on narrow terminals + +When the terminal width is less than 80 columns, the renderer SHALL **auto-collapse** the top-3 open-findings list (the `Open findings:` header and its rows) out of the digest, while still rendering the `Decision:`, `Findings:`, `Severity:`, and `Latest summary:` lines. + +Under the same narrow-terminal condition, any single remaining digest line that would exceed the terminal width SHALL be truncated with a trailing ellipsis character `…` rather than wrapped across multiple terminal rows. The ellipsis SHALL be the final character of the truncated line; no further characters SHALL appear after it. + +When the terminal width is 80 columns or wider, all digest lines SHALL render in full, and the `Open findings:` list SHALL render per the ranking rules defined above. + +#### Scenario: Findings list collapses below 80 columns + +- **WHEN** the terminal width is 79 columns and the latest snapshot has three open findings +- **THEN** the digest SHALL render the `Decision:`, `Findings:`, `Severity:`, and `Latest summary:` lines +- **AND** the `Open findings:` header and its three rows SHALL NOT render + +#### Scenario: Non-findings digest lines truncate with ellipsis below 80 columns + +- **WHEN** the terminal width is 60 columns and the `Latest summary:` line would be 120 characters at full width +- **THEN** the rendered `Latest summary:` line SHALL be truncated to at most 60 characters including a trailing `…` +- **AND** the truncation SHALL NOT produce a wrapped second row + +#### Scenario: Wide terminals render the full digest + +- **WHEN** the terminal width is 120 columns and the latest snapshot has five open findings +- **THEN** the digest SHALL render all lines in full +- **AND** the `Open findings:` block SHALL show exactly three rows ranked per the ranking rules + +## MODIFIED Requirements + +### Requirement: UI consumes only read-only local artifacts + +The system SHALL consume the following artifact contracts in read-only mode and SHALL NOT write to any run artifact: + +- the run-state JSON defined by `run-artifact-store-conformance`; +- the autofix progress snapshot defined by `review-autofix-progress-observability`; +- the observation events log defined by `workflow-observation-events`; +- the `task-graph.json` file at `openspec/changes/<run.change_name>/task-graph.json`; +- the review ledger JSON for the active review family, defined by `review-orchestration`, at one of `openspec/changes/<run.change_name>/review-ledger-design.json` or `openspec/changes/<run.change_name>/review-ledger.json` per the family rule. + +The UI SHALL NOT watch or render content from `tasks.md`. + +Multiple concurrent `specflow-watch` processes on the same run SHALL be safe (read-only, no lock contention). + +#### Scenario: Watcher does not write to run artifacts + +- **WHEN** `specflow-watch` is running against a run +- **THEN** no watched artifact file on disk SHALL be modified by the `specflow-watch` process +- **AND** `specflow-watch` SHALL NOT call any `specflow-run advance` or other mutating specflow subcommand + +#### Scenario: task-graph path is derived from run.change_name + +- **WHEN** `specflow-watch` resolves the tracked run +- **THEN** it SHALL locate the task-graph at `openspec/changes/<run.change_name>/task-graph.json` +- **AND** it SHALL NOT read or watch `tasks.md` for task progress + +#### Scenario: Review ledger path is derived from run.change_name and active family + +- **WHEN** `specflow-watch` resolves the tracked run and `current_phase` maps to the design review family +- **THEN** it SHALL locate the ledger at `openspec/changes/<run.change_name>/review-ledger-design.json` +- **AND WHEN** `current_phase` maps to the apply review family +- **THEN** it SHALL locate the ledger at `openspec/changes/<run.change_name>/review-ledger.json` +- **AND** neither ledger SHALL be written by the watcher process + +#### Scenario: Multiple concurrent watchers coexist + +- **WHEN** two or more `specflow-watch` processes are started against the same `run_id` +- **THEN** each process SHALL render independently without interfering with the others or with the run's producers + +### Requirement: Graceful degradation per section + +The UI SHALL degrade gracefully when a non-essential source is missing, unparseable, or not yet applicable. Run-state is the only mandatory source. + +Specifically: + +- If `task-graph.json` does not exist for the tracked run, the task-graph section SHALL display a placeholder such as "No task graph yet (generated in design phase)". +- If no active autofix snapshot exists, the review round section SHALL display a placeholder such as "No active review". +- If the observation events log has no entries for the tracked run, the recent events section SHALL display a placeholder such as "No events recorded". +- If the review-family ledger file is absent, the Review section's digest layer SHALL display the placeholder `No review digest yet` below the existing progress view; the snapshot-based progress view SHALL continue to render when a snapshot exists. +- If the review-family ledger file exists but is unreadable (I/O failure), the Review section's digest layer SHALL display the inline warning line `Review ledger unreadable: <reason>` with `<reason>` drawn from the reader status; the snapshot-based progress view and all other sections SHALL continue to render normally. +- If the review-family ledger file exists but is malformed (parse failure), the Review section's digest layer SHALL display an inline warning line identifying the source and the parse problem; the snapshot-based progress view and all other sections SHALL continue to render normally. +- If the review-family ledger file exists and is parseable but contains zero `LedgerSnapshot` entries, the digest layer SHALL display the same `No review digest yet` placeholder used for the absent-file case. +- If any other non-essential source is unparseable or malformed, the affected section SHALL display an inline warning, and the other sections SHALL continue to render. +- If the run-state for the resolved run cannot be read (missing or unparseable), the CLI SHALL exit with a non-zero status and a clear error message rather than render an incomplete TUI. + +#### Scenario: Missing task-graph shows placeholder + +- **WHEN** `openspec/changes/<run.change_name>/task-graph.json` does not exist at watcher start or at redraw time +- **THEN** the task-graph section SHALL display a placeholder indicating no task graph yet +- **AND** the other sections SHALL continue to render normally + +#### Scenario: Missing autofix snapshot shows placeholder + +- **WHEN** no autofix progress snapshot exists for the tracked run +- **THEN** the review round section SHALL display a placeholder indicating no active review +- **AND** the other sections SHALL continue to render normally + +#### Scenario: Missing ledger shows digest placeholder + +- **WHEN** the active review family maps to a ledger file that does not exist on disk +- **THEN** the Review section's digest layer SHALL display `No review digest yet` below the existing progress view +- **AND** the snapshot-based progress view SHALL continue to render when a snapshot exists +- **AND** all other sections SHALL render normally + +#### Scenario: Unreadable ledger shows inline warning + +- **WHEN** the active review family's ledger file exists but cannot be read (I/O failure, permission error) +- **THEN** the Review section's digest layer SHALL display `Review ledger unreadable: <reason>` +- **AND** the snapshot-based progress view SHALL continue to render when a snapshot exists +- **AND** all other sections SHALL render normally + +#### Scenario: Malformed ledger shows inline warning + +- **WHEN** the active review family's ledger file exists but cannot be parsed as JSON or does not conform to the ledger schema +- **THEN** the Review section's digest layer SHALL display an inline warning identifying the ledger source and the parse problem +- **AND** the snapshot-based progress view SHALL continue to render when a snapshot exists +- **AND** all other sections SHALL render normally + +#### Scenario: Empty ledger shows digest placeholder + +- **WHEN** the active review family's ledger file exists, is parseable, and contains zero `LedgerSnapshot` entries +- **THEN** the Review section's digest layer SHALL display `No review digest yet` +- **AND** no `Decision:` / `Findings:` / `Severity:` lines SHALL render + +#### Scenario: Malformed source shows inline warning + +- **WHEN** one of the watched source files exists but cannot be parsed +- **THEN** the affected section SHALL show an inline warning identifying the source and the parse problem +- **AND** the other sections SHALL continue to render normally + +#### Scenario: Missing run-state aborts startup + +- **WHEN** the resolved run has no readable run-state record +- **THEN** the CLI SHALL exit with a non-zero status and a clear error message +- **AND** the CLI SHALL NOT render a partial TUI diff --git a/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/task-graph.json b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/task-graph.json new file mode 100644 index 0000000..5323f39 --- /dev/null +++ b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/task-graph.json @@ -0,0 +1,221 @@ +{ + "version": "1.0", + "change_id": "extend-specflow-watch-with-review-ledger-digests-and-subagent-activity", + "bundles": [ + { + "id": "lock-review-ledger-digest-contract", + "title": "Lock Review Ledger Digest Contract", + "goal": "Align the delta spec and implementation checklist around digest content, graceful degradation, and the watcher's read-only boundary before code wiring begins.", + "depends_on": [], + "inputs": [ + "openspec/changes/extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/design.md", + "openspec/changes/extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/specs/realtime-progress-ui/spec.md", + "specs/review-autofix-progress-observability", + "specs/review-orchestration", + "specs/artifact-ownership-model", + "specs/run-artifact-store-conformance" + ], + "outputs": [ + "validated realtime-progress-ui digest and degradation delta", + "implementation checklist for Phase 1 summary fallback, empty-ledger placeholder, and no-new-events scope" + ], + "status": "done", + "tasks": [ + { + "id": "1", + "title": "Reconcile the design's digest fields, ranking order, and narrow-terminal behavior with the realtime-progress-ui delta scenarios", + "status": "done" + }, + { + "id": "2", + "title": "Confirm the read-only artifact contract covers family-mapped review-ledger paths and excludes all watcher writes and mutating review-ledger helpers", + "status": "done" + }, + { + "id": "3", + "title": "Freeze the Phase 1 handling for latest-summary fallback and empty-ledger placeholder semantics in an implementation checklist", + "status": "done" + }, + { + "id": "4", + "title": "Confirm subagent activity and new observation events remain out of scope for this change", + "status": "done" + } + ], + "owner_capabilities": [ + "realtime-progress-ui", + "review-autofix-progress-observability", + "review-orchestration", + "artifact-ownership-model", + "run-artifact-store-conformance", + "spec-consistency-verification" + ], + "size_score": 4 + }, + { + "id": "add-ledger-readers-and-family-selection", + "title": "Add Ledger Readers And Family Selection", + "goal": "Provide tolerant review-ledger readers and a phase-to-ledger-family selector in the watcher's artifact layer.", + "depends_on": [ + "lock-review-ledger-digest-contract" + ], + "inputs": [ + "validated realtime-progress-ui digest and degradation delta", + "implementation checklist for Phase 1 summary fallback, empty-ledger placeholder, and no-new-events scope", + "src/lib/specflow-watch/artifact-readers.ts", + "src/types/contracts.ts", + "src/tests/specflow-watch-readers.test.ts" + ], + "outputs": [ + "src/lib/specflow-watch/artifact-readers.ts", + "src/tests/specflow-watch-readers.test.ts", + "tolerant design/apply review-ledger readers and selectActiveReviewLedger" + ], + "status": "done", + "tasks": [ + { + "id": "1", + "title": "Add design and apply review-ledger path helpers plus readDesignReviewLedger and readApplyReviewLedger returning ok, absent, unreadable, or malformed", + "status": "done" + }, + { + "id": "2", + "title": "Implement a watcher-local ledger validator that only requires digest fields and tolerates unknown extras without side effects", + "status": "done" + }, + { + "id": "3", + "title": "Add selectActiveReviewLedger(currentPhase) and verify its family mapping stays in parity with selectActiveAutofixPhase", + "status": "done" + }, + { + "id": "4", + "title": "Extend specflow-watch reader tests for ok, absent, unreadable, malformed, and empty-round-summaries ledger cases", + "status": "done" + } + ], + "owner_capabilities": [ + "realtime-progress-ui", + "review-orchestration", + "artifact-ownership-model", + "phase-semantics" + ], + "size_score": 4 + }, + { + "id": "build-and-render-review-ledger-digest", + "title": "Build And Render Review Ledger Digest", + "goal": "Derive a review-ledger digest from the latest persisted ledger state and render it compactly beneath the existing snapshot progress lines.", + "depends_on": [ + "lock-review-ledger-digest-contract", + "add-ledger-readers-and-family-selection" + ], + "inputs": [ + "validated realtime-progress-ui digest and degradation delta", + "tolerant design/apply review-ledger readers and selectActiveReviewLedger", + "src/lib/watch-renderer/model.ts", + "src/lib/watch-renderer/render.ts", + "src/lib/watch-renderer/index.ts", + "src/types/contracts.ts", + "src/tests/watch-renderer.test.ts" + ], + "outputs": [ + "src/lib/watch-renderer/model.ts", + "src/lib/watch-renderer/render.ts", + "src/lib/watch-renderer/index.ts", + "src/tests/watch-renderer.test.ts", + "LedgerDigest model and digest rendering behavior" + ], + "status": "done", + "tasks": [ + { + "id": "1", + "title": "Extend the review section model with LedgerDigest or null plus placeholder and warning states derived from ledger reader results", + "status": "done" + }, + { + "id": "2", + "title": "Build digest decision, counts, open-severity totals, latest-summary fallback, and top-3 ranking from the last round_summaries entry plus open findings", + "status": "done" + }, + { + "id": "3", + "title": "Render digest lines below snapshot progress without changing existing snapshot and manual-fix behavior", + "status": "done" + }, + { + "id": "4", + "title": "Apply sub-80-column findings collapse and ellipsis truncation in the renderer only, keeping terminal width out of the model", + "status": "done" + }, + { + "id": "5", + "title": "Expand watch-renderer tests for populated digests, ranking tie-breakers, empty ledgers, warning states, and narrow versus wide terminal output", + "status": "done" + } + ], + "owner_capabilities": [ + "realtime-progress-ui", + "review-orchestration", + "review-autofix-progress-observability", + "phase-semantics" + ], + "size_score": 5 + }, + { + "id": "wire-ledger-digest-through-specflow-watch", + "title": "Wire Ledger Digest Through Specflow Watch", + "goal": "Connect the new ledger readers and digest model through specflow-watch, including watched ledger paths and end-to-end redraw coverage.", + "depends_on": [ + "add-ledger-readers-and-family-selection", + "build-and-render-review-ledger-digest" + ], + "inputs": [ + "tolerant design/apply review-ledger readers and selectActiveReviewLedger", + "LedgerDigest model and digest rendering behavior", + "src/bin/specflow-watch.ts", + "src/tests/specflow-watch-integration.test.ts", + "src/tests/specflow-watch-import-graph.test.ts" + ], + "outputs": [ + "src/bin/specflow-watch.ts", + "src/tests/specflow-watch-integration.test.ts", + "design/apply ledger watch-path wiring and redraw coverage", + "read-only import-graph verification for ledger-digest wiring" + ], + "status": "done", + "tasks": [ + { + "id": "1", + "title": "Wire active ledger selection and tolerant ledger reads into buildModel and review-section construction in src/bin/specflow-watch.ts", + "status": "done" + }, + { + "id": "2", + "title": "Add design and apply review-ledger files to the watched path set and keep redraw behavior aligned with the existing polling fallback", + "status": "done" + }, + { + "id": "3", + "title": "Extend specflow-watch integration tests to cover initial digest render and redraw after ledger updates on disk", + "status": "done" + }, + { + "id": "4", + "title": "Re-check the import-graph guard so the new wiring does not pull in mutating review-ledger code or write-oriented APIs", + "status": "done" + } + ], + "owner_capabilities": [ + "realtime-progress-ui", + "workflow-run-state", + "workflow-observation-events", + "review-autofix-progress-observability", + "artifact-ownership-model" + ], + "size_score": 4 + } + ], + "generated_at": "2026-04-24T13:00:05.871Z", + "generated_from": "design.md" +} diff --git a/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/tasks.md b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/tasks.md new file mode 100644 index 0000000..20b5887 --- /dev/null +++ b/openspec/changes/archive/2026-04-25-extend-specflow-watch-with-review-ledger-digests-and-subagent-activity/tasks.md @@ -0,0 +1,42 @@ +## 1. Lock Review Ledger Digest Contract ✓ + +> Align the delta spec and implementation checklist around digest content, graceful degradation, and the watcher's read-only boundary before code wiring begins. + +- [x] 1.1 Reconcile the design's digest fields, ranking order, and narrow-terminal behavior with the realtime-progress-ui delta scenarios +- [x] 1.2 Confirm the read-only artifact contract covers family-mapped review-ledger paths and excludes all watcher writes and mutating review-ledger helpers +- [x] 1.3 Freeze the Phase 1 handling for latest-summary fallback and empty-ledger placeholder semantics in an implementation checklist +- [x] 1.4 Confirm subagent activity and new observation events remain out of scope for this change + +## 2. Add Ledger Readers And Family Selection ✓ + +> Provide tolerant review-ledger readers and a phase-to-ledger-family selector in the watcher's artifact layer. + +> Depends on: lock-review-ledger-digest-contract + +- [x] 2.1 Add design and apply review-ledger path helpers plus readDesignReviewLedger and readApplyReviewLedger returning ok, absent, unreadable, or malformed +- [x] 2.2 Implement a watcher-local ledger validator that only requires digest fields and tolerates unknown extras without side effects +- [x] 2.3 Add selectActiveReviewLedger(currentPhase) and verify its family mapping stays in parity with selectActiveAutofixPhase +- [x] 2.4 Extend specflow-watch reader tests for ok, absent, unreadable, malformed, and empty-round-summaries ledger cases + +## 3. Build And Render Review Ledger Digest ✓ + +> Derive a review-ledger digest from the latest persisted ledger state and render it compactly beneath the existing snapshot progress lines. + +> Depends on: lock-review-ledger-digest-contract, add-ledger-readers-and-family-selection + +- [x] 3.1 Extend the review section model with LedgerDigest or null plus placeholder and warning states derived from ledger reader results +- [x] 3.2 Build digest decision, counts, open-severity totals, latest-summary fallback, and top-3 ranking from the last round_summaries entry plus open findings +- [x] 3.3 Render digest lines below snapshot progress without changing existing snapshot and manual-fix behavior +- [x] 3.4 Apply sub-80-column findings collapse and ellipsis truncation in the renderer only, keeping terminal width out of the model +- [x] 3.5 Expand watch-renderer tests for populated digests, ranking tie-breakers, empty ledgers, warning states, and narrow versus wide terminal output + +## 4. Wire Ledger Digest Through Specflow Watch ✓ + +> Connect the new ledger readers and digest model through specflow-watch, including watched ledger paths and end-to-end redraw coverage. + +> Depends on: add-ledger-readers-and-family-selection, build-and-render-review-ledger-digest + +- [x] 4.1 Wire active ledger selection and tolerant ledger reads into buildModel and review-section construction in src/bin/specflow-watch.ts +- [x] 4.2 Add design and apply review-ledger files to the watched path set and keep redraw behavior aligned with the existing polling fallback +- [x] 4.3 Extend specflow-watch integration tests to cover initial digest render and redraw after ledger updates on disk +- [x] 4.4 Re-check the import-graph guard so the new wiring does not pull in mutating review-ledger code or write-oriented APIs diff --git a/openspec/specs/realtime-progress-ui/spec.md b/openspec/specs/realtime-progress-ui/spec.md index 520c6e5..1b359a8 100644 --- a/openspec/specs/realtime-progress-ui/spec.md +++ b/openspec/specs/realtime-progress-ui/spec.md @@ -58,7 +58,8 @@ The system SHALL consume the following artifact contracts in read-only mode and - the run-state JSON defined by `run-artifact-store-conformance`; - the autofix progress snapshot defined by `review-autofix-progress-observability`; - the observation events log defined by `workflow-observation-events`; -- the `task-graph.json` file at `openspec/changes/<run.change_name>/task-graph.json`. +- the `task-graph.json` file at `openspec/changes/<run.change_name>/task-graph.json`; +- the review ledger JSON for the active review family, defined by `review-orchestration`, at one of `openspec/changes/<run.change_name>/review-ledger-design.json` or `openspec/changes/<run.change_name>/review-ledger.json` per the family rule. The UI SHALL NOT watch or render content from `tasks.md`. @@ -76,6 +77,14 @@ Multiple concurrent `specflow-watch` processes on the same run SHALL be safe (re - **THEN** it SHALL locate the task-graph at `openspec/changes/<run.change_name>/task-graph.json` - **AND** it SHALL NOT read or watch `tasks.md` for task progress +#### Scenario: Review ledger path is derived from run.change_name and active family + +- **WHEN** `specflow-watch` resolves the tracked run and `current_phase` maps to the design review family +- **THEN** it SHALL locate the ledger at `openspec/changes/<run.change_name>/review-ledger-design.json` +- **AND WHEN** `current_phase` maps to the apply review family +- **THEN** it SHALL locate the ledger at `openspec/changes/<run.change_name>/review-ledger.json` +- **AND** neither ledger SHALL be written by the watcher process + #### Scenario: Multiple concurrent watchers coexist - **WHEN** two or more `specflow-watch` processes are started against the same `run_id` @@ -124,7 +133,11 @@ Specifically: - If `task-graph.json` does not exist for the tracked run, the task-graph section SHALL display a placeholder such as "No task graph yet (generated in design phase)". - If no active autofix snapshot exists, the review round section SHALL display a placeholder such as "No active review". - If the observation events log has no entries for the tracked run, the recent events section SHALL display a placeholder such as "No events recorded". -- If any non-essential source is unparseable or malformed, the affected section SHALL display an inline warning, and the other sections SHALL continue to render. +- If the review-family ledger file is absent, the Review section's digest layer SHALL display the placeholder `No review digest yet` below the existing progress view; the snapshot-based progress view SHALL continue to render when a snapshot exists. +- If the review-family ledger file exists but is unreadable (I/O failure), the Review section's digest layer SHALL display the inline warning line `Review ledger unreadable: <reason>` with `<reason>` drawn from the reader status; the snapshot-based progress view and all other sections SHALL continue to render normally. +- If the review-family ledger file exists but is malformed (parse failure), the Review section's digest layer SHALL display an inline warning line identifying the source and the parse problem; the snapshot-based progress view and all other sections SHALL continue to render normally. +- If the review-family ledger file exists and is parseable but contains zero `LedgerSnapshot` entries, the digest layer SHALL display the same `No review digest yet` placeholder used for the absent-file case. +- If any other non-essential source is unparseable or malformed, the affected section SHALL display an inline warning, and the other sections SHALL continue to render. - If the run-state for the resolved run cannot be read (missing or unparseable), the CLI SHALL exit with a non-zero status and a clear error message rather than render an incomplete TUI. #### Scenario: Missing task-graph shows placeholder @@ -139,6 +152,33 @@ Specifically: - **THEN** the review round section SHALL display a placeholder indicating no active review - **AND** the other sections SHALL continue to render normally +#### Scenario: Missing ledger shows digest placeholder + +- **WHEN** the active review family maps to a ledger file that does not exist on disk +- **THEN** the Review section's digest layer SHALL display `No review digest yet` below the existing progress view +- **AND** the snapshot-based progress view SHALL continue to render when a snapshot exists +- **AND** all other sections SHALL render normally + +#### Scenario: Unreadable ledger shows inline warning + +- **WHEN** the active review family's ledger file exists but cannot be read (I/O failure, permission error) +- **THEN** the Review section's digest layer SHALL display `Review ledger unreadable: <reason>` +- **AND** the snapshot-based progress view SHALL continue to render when a snapshot exists +- **AND** all other sections SHALL render normally + +#### Scenario: Malformed ledger shows inline warning + +- **WHEN** the active review family's ledger file exists but cannot be parsed as JSON or does not conform to the ledger schema +- **THEN** the Review section's digest layer SHALL display an inline warning identifying the ledger source and the parse problem +- **AND** the snapshot-based progress view SHALL continue to render when a snapshot exists +- **AND** all other sections SHALL render normally + +#### Scenario: Empty ledger shows digest placeholder + +- **WHEN** the active review family's ledger file exists, is parseable, and contains zero `LedgerSnapshot` entries +- **THEN** the Review section's digest layer SHALL display `No review digest yet` +- **AND** no `Decision:` / `Findings:` / `Severity:` lines SHALL render + #### Scenario: Malformed source shows inline warning - **WHEN** one of the watched source files exists but cannot be parsed @@ -373,3 +413,91 @@ The section SHALL be watched for filesystem changes so that a later `approval-su - **WHEN** `approval-summary.md` is written or updated while the watcher is running - **THEN** the Approval summary section SHALL redraw on the next filesystem event or polling cycle +### Requirement: Review section renders a ledger digest below snapshot progress + +The Review section of the TUI SHALL render a **ledger digest layer** below the existing snapshot-based progress lines whenever a review-ledger file matching the active review family is available. The digest SHALL NOT replace the existing snapshot-derived progress view; both layers coexist. + +Family-to-ledger mapping SHALL match the snapshot family rule already defined for the Review round section: + +- `current_phase ∈ {design_draft, design_review, design_ready}` → read `openspec/changes/<run.change_name>/review-ledger-design.json`. +- `current_phase ∈ {apply_draft, apply_review, apply_ready, approved}` → read `openspec/changes/<run.change_name>/review-ledger.json`. +- Any other phase → no ledger digest is rendered for this run. + +When a ledger is readable, the digest SHALL be built from the **latest `LedgerSnapshot`** only (no cross-round aggregation), and SHALL contain, in this order: + +1. `Decision: <decision>` — the latest snapshot's decision verbatim (e.g. `approve_with_findings`). If the snapshot has no decision value, the renderer SHALL display `Decision: (none)`. +2. `Findings: <total> total | <open> open | <new> new | <resolved> resolved` — counts derived from the latest snapshot. +3. `Severity: HIGH <h> | MEDIUM <m> | LOW <l>` — computed **over open findings only** (`status ∈ {open, new}` in the latest snapshot). Resolved and overridden findings SHALL NOT contribute to this breakdown. +4. `Latest summary: <text>` — the latest round-summary text from the snapshot. If absent, the line SHALL be omitted. +5. `Open findings:` — a header line followed by up to three rows listing the top unresolved findings as `<SEVERITY> <title>`. Exactly three findings SHALL be shown when at least three are open; fewer rows are shown when fewer open findings exist. The header line and its rows SHALL be omitted entirely when zero open findings exist. + +Ranking of the open findings list SHALL be: + +- Primary sort: severity, in the order `HIGH` > `MEDIUM` > `LOW`. +- Secondary sort (same severity): `latest_round` DESC — findings touched in the most recent round first. +- Tertiary sort (same severity and same `latest_round`): finding `id` ASC, interpreted as a stable string comparison. + +The digest SHALL remain visible in the adjacent completed-family phases (`design_ready`, `apply_ready`, `approved`) consistent with the existing family-based review-section visibility rule. + +#### Scenario: Digest renders under snapshot progress during apply_review + +- **WHEN** `current_phase == "apply_review"`, an `autofix-progress-apply_review.json` snapshot exists, and `review-ledger.json` contains at least one `LedgerSnapshot` +- **THEN** the Review section SHALL render the existing snapshot progress lines unchanged +- **AND** below those lines it SHALL render a `Decision: …` line, a `Findings: … total | … open | … new | … resolved` line, a `Severity: HIGH … | MEDIUM … | LOW …` line, a `Latest summary: …` line, and an `Open findings:` block drawn from the **latest** `LedgerSnapshot` + +#### Scenario: Severity breakdown only counts open findings + +- **WHEN** the latest snapshot contains findings with mixed statuses (for example, 1 resolved HIGH, 2 open HIGH, 1 open MEDIUM, 1 overridden LOW) +- **THEN** the `Severity:` line SHALL display `HIGH 2 | MEDIUM 1 | LOW 0` +- **AND** resolved and overridden findings SHALL NOT contribute to the severity counts + +#### Scenario: Open findings list ranks by severity then latest_round then id + +- **WHEN** the latest snapshot contains five open findings with the following `(severity, latest_round, id)` tuples: `(MEDIUM, 4, R4-F01)`, `(HIGH, 2, R2-F03)`, `(HIGH, 3, R3-F01)`, `(LOW, 5, R5-F02)`, `(HIGH, 3, R3-F02)` +- **THEN** the `Open findings:` block SHALL render exactly three rows +- **AND** the rows SHALL be, in order: the `HIGH` at `latest_round=3, id=R3-F01`, the `HIGH` at `latest_round=3, id=R3-F02`, then the `HIGH` at `latest_round=2, id=R2-F03` +- **AND** the `MEDIUM` and `LOW` findings SHALL NOT appear in the block + +#### Scenario: Digest remains visible on apply_ready after review completes + +- **WHEN** `current_phase == "apply_ready"` and `review-ledger.json` contains a terminal `LedgerSnapshot` +- **THEN** the Review section SHALL render the digest below the completed snapshot progress view +- **AND** the digest SHALL reflect the latest `LedgerSnapshot` + +#### Scenario: Digest is suppressed on non-review phases + +- **WHEN** `current_phase` is `spec_draft`, `proposal_clarify`, or any other non-review-family phase +- **THEN** the Review section SHALL NOT read or render a ledger digest regardless of whether `review-ledger.json` or `review-ledger-design.json` exists on disk + +#### Scenario: Latest round summary missing elides the line + +- **WHEN** the latest snapshot has no round-summary text +- **THEN** the renderer SHALL omit the `Latest summary:` line entirely +- **AND** the other digest lines SHALL still render + +### Requirement: Digest degrades compactly on narrow terminals + +When the terminal width is less than 80 columns, the renderer SHALL **auto-collapse** the top-3 open-findings list (the `Open findings:` header and its rows) out of the digest, while still rendering the `Decision:`, `Findings:`, `Severity:`, and `Latest summary:` lines. + +Under the same narrow-terminal condition, any single remaining digest line that would exceed the terminal width SHALL be truncated with a trailing ellipsis character `…` rather than wrapped across multiple terminal rows. The ellipsis SHALL be the final character of the truncated line; no further characters SHALL appear after it. + +When the terminal width is 80 columns or wider, all digest lines SHALL render in full, and the `Open findings:` list SHALL render per the ranking rules defined above. + +#### Scenario: Findings list collapses below 80 columns + +- **WHEN** the terminal width is 79 columns and the latest snapshot has three open findings +- **THEN** the digest SHALL render the `Decision:`, `Findings:`, `Severity:`, and `Latest summary:` lines +- **AND** the `Open findings:` header and its three rows SHALL NOT render + +#### Scenario: Non-findings digest lines truncate with ellipsis below 80 columns + +- **WHEN** the terminal width is 60 columns and the `Latest summary:` line would be 120 characters at full width +- **THEN** the rendered `Latest summary:` line SHALL be truncated to at most 60 characters including a trailing `…` +- **AND** the truncation SHALL NOT produce a wrapped second row + +#### Scenario: Wide terminals render the full digest + +- **WHEN** the terminal width is 120 columns and the latest snapshot has five open findings +- **THEN** the digest SHALL render all lines in full +- **AND** the `Open findings:` block SHALL show exactly three rows ranked per the ranking rules + diff --git a/src/bin/specflow-watch.ts b/src/bin/specflow-watch.ts index 1e3f66f..ed1610a 100644 --- a/src/bin/specflow-watch.ts +++ b/src/bin/specflow-watch.ts @@ -20,10 +20,13 @@ import { phaseIsLiveReviewGate, readApprovalSummary, readAutofixSnapshotFile, + readReviewLedgerFile, readRunStateFile, readTaskGraphFile, + reviewLedgerPath, runStatePath, selectActiveAutofixPhase, + selectActiveReviewLedger, taskGraphPath, } from "../lib/specflow-watch/artifact-readers.js"; import { resolveTrackedRun } from "../lib/specflow-watch/run-resolution.js"; @@ -34,6 +37,7 @@ import { ALT_SCREEN_ENTER, ALT_SCREEN_LEAVE, buildApprovalSummary, + buildDigestState, buildEventsView, buildHeader, buildReviewView, @@ -137,6 +141,18 @@ function buildModel(inputs: ModelInputs): WatchModel { manual_fix_kind: manualFixKind, }); + // Ledger digest layer (independent from snapshot view). + const activeLedgerFamily = selectActiveReviewLedger(phase); + const changeForLedger = run.change_name ?? ""; + const ledgerRead = + activeLedgerFamily === null || !changeForLedger + ? ({ kind: "absent" } as const) + : readReviewLedgerFile(repoRoot, changeForLedger, activeLedgerFamily); + const digest = buildDigestState({ + activeFamily: activeLedgerFamily, + ledgerRead, + }); + // Task graph const changeForGraph = run.change_name ?? ""; const graphRead = changeForGraph @@ -165,6 +181,7 @@ function buildModel(inputs: ModelInputs): WatchModel { header, terminal_banner: runRead.kind === "ok" ? terminalBannerFor(status) : null, review, + digest, task_graph: taskGraphView, events: eventsView, approval_summary: approvalSummary, @@ -319,6 +336,10 @@ function main(): void { ]; if (run.change_name) { watchedPaths.push(taskGraphPath(repoRoot, run.change_name)); + // Watch both review-ledger files so a missing→present transition triggers + // a redraw. The polling fallback also catches atomic-write transitions. + watchedPaths.push(reviewLedgerPath(repoRoot, run.change_name, "design")); + watchedPaths.push(reviewLedgerPath(repoRoot, run.change_name, "apply")); } // Watch the directory containing approval-summary.md so a missing→present // transition (first approve) triggers a redraw. Falls back to the file diff --git a/src/lib/specflow-watch/artifact-readers.ts b/src/lib/specflow-watch/artifact-readers.ts index 4bec7be..892c689 100644 --- a/src/lib/specflow-watch/artifact-readers.ts +++ b/src/lib/specflow-watch/artifact-readers.ts @@ -9,7 +9,12 @@ import { existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; import type { AutofixProgressSnapshot } from "../../types/autofix-progress.js"; import { validateAutofixSnapshot } from "../../types/autofix-progress.js"; -import type { RunState } from "../../types/contracts.js"; +import type { + LedgerRoundSummary, + ReviewFinding, + ReviewLedger, + RunState, +} from "../../types/contracts.js"; import type { TaskGraph } from "../task-planner/index.js"; import { validateTaskGraph } from "../task-planner/index.js"; @@ -135,6 +140,288 @@ export function phaseIsLiveReviewGate(currentPhase: string): boolean { return currentPhase === "design_review" || currentPhase === "apply_review"; } +// --------------------------------------------------------------------------- +// Review ledger (per change — design / apply families) +// --------------------------------------------------------------------------- + +export type ReviewLedgerFamily = "design" | "apply"; + +export function reviewLedgerPath( + projectRoot: string, + changeName: string, + family: ReviewLedgerFamily, +): string { + const filename = + family === "design" ? "review-ledger-design.json" : "review-ledger.json"; + return join(projectRoot, "openspec/changes", changeName, filename); +} + +/** + * Phase-to-ledger-family selector. Mirrors `selectActiveAutofixPhase` so + * snapshot and digest stay phase-consistent. Returns `null` for non-review + * families so the caller can render the digest as `hidden`. + */ +export function selectActiveReviewLedger( + currentPhase: string, +): ReviewLedgerFamily | null { + switch (currentPhase) { + case "design_draft": + case "design_review": + case "design_ready": + return "design"; + case "apply_draft": + case "apply_review": + case "apply_ready": + case "approved": + return "apply"; + default: + return null; + } +} + +function isString(value: unknown): value is string { + return typeof value === "string"; +} + +function isFiniteNumber(value: unknown): value is number { + return typeof value === "number" && Number.isFinite(value); +} + +function isObject(value: unknown): value is Record<string, unknown> { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function validateReviewFinding(finding: unknown, index: number): string | null { + if (!isObject(finding)) return `findings[${index}]: not an object`; + // Mandatory string fields the digest depends on — reject if missing. + for (const field of ["id", "title", "severity", "status"] as const) { + if (!isString(finding[field])) { + return `findings[${index}].${field}: missing or not a string`; + } + } + // Optional string fields — type-check only when present. + for (const field of [ + "file", + "category", + "detail", + "relation", + "notes", + ] as const) { + if ( + field in finding && + !isString(finding[field]) && + finding[field] !== null + ) { + return `findings[${index}].${field}: not a string`; + } + } + for (const field of [ + "origin_round", + "latest_round", + "resolved_round", + ] as const) { + if (field in finding && !isFiniteNumber(finding[field])) { + return `findings[${index}].${field}: not a number`; + } + } + if ( + "supersedes" in finding && + finding.supersedes !== null && + !isString(finding.supersedes) + ) { + return `findings[${index}].supersedes: not string|null`; + } + return null; +} + +function validateRoundSummary(round: unknown, index: number): string | null { + if (!isObject(round)) return `round_summaries[${index}]: not an object`; + for (const field of [ + "round", + "total", + "open", + "new", + "resolved", + "overridden", + ] as const) { + if (!isFiniteNumber(round[field])) { + return `round_summaries[${index}].${field}: missing or not a number`; + } + } + if (!isObject(round.by_severity)) { + return `round_summaries[${index}].by_severity: not an object`; + } + for (const [k, v] of Object.entries(round.by_severity)) { + if (!isFiniteNumber(v)) { + return `round_summaries[${index}].by_severity[${k}]: not a number`; + } + } + for (const field of [ + "decision", + "proposal_hash", + "blocking_signature", + ] as const) { + if (field in round && !isString(round[field])) { + return `round_summaries[${index}].${field}: not a string`; + } + } + for (const field of [ + "blocking_count", + "stagnant_rounds", + "max_rounds", + ] as const) { + if (field in round && !isFiniteNumber(round[field])) { + return `round_summaries[${index}].${field}: not a number`; + } + } + if ( + "stop_reason" in round && + round.stop_reason !== null && + !isString(round.stop_reason) + ) { + return `round_summaries[${index}].stop_reason: not string|null`; + } + if ( + "gate_id" in round && + round.gate_id !== null && + !isString(round.gate_id) + ) { + return `round_summaries[${index}].gate_id: not string|null`; + } + return null; +} + +/** + * Validates a parsed JSON value against the full `ReviewLedger` contract. + * Returns a non-empty reason string when the value violates the schema. + * Side-effect free — does not rename, copy, or back up files. + */ +export function validateReviewLedgerSchema(value: unknown): string | null { + if (!isObject(value)) return "ledger: not an object"; + for (const field of ["feature_id", "phase", "status"] as const) { + if (!isString(value[field])) { + return `${field}: missing or not a string`; + } + } + for (const field of ["current_round", "max_finding_id"] as const) { + if (!isFiniteNumber(value[field])) { + return `${field}: missing or not a number`; + } + } + if (!Array.isArray(value.findings)) { + return "findings: missing or not an array"; + } + for (let i = 0; i < value.findings.length; i++) { + const reason = validateReviewFinding(value.findings[i], i); + if (reason) return reason; + } + if (!Array.isArray(value.round_summaries)) { + return "round_summaries: missing or not an array"; + } + for (let i = 0; i < value.round_summaries.length; i++) { + const reason = validateRoundSummary(value.round_summaries[i], i); + if (reason) return reason; + } + for (const field of [ + "latest_decision", + "proposal_hash", + "blocking_signature", + ] as const) { + if (field in value && !isString(value[field])) { + return `${field}: not a string`; + } + } + for (const field of [ + "blocking_count", + "stagnant_rounds", + "max_rounds", + ] as const) { + if (field in value && !isFiniteNumber(value[field])) { + return `${field}: not a number`; + } + } + if ( + "stop_reason" in value && + value.stop_reason !== null && + !isString(value.stop_reason) + ) { + return "stop_reason: not string|null"; + } + + // Decision parity: when both `latest_decision` and the last round summary's + // `decision` are present, they must agree. Silent disagreement would render + // stale or inconsistent decision data — treat as malformed. + const latestDecision = isString(value.latest_decision) + ? value.latest_decision + : null; + const summaries = value.round_summaries as readonly LedgerRoundSummary[]; + const lastRoundDecision = + summaries.length > 0 && isString(summaries[summaries.length - 1].decision) + ? summaries[summaries.length - 1].decision + : null; + if ( + latestDecision !== null && + lastRoundDecision !== null && + latestDecision !== lastRoundDecision + ) { + return `decision parity violation: latest_decision="${latestDecision}" disagrees with round_summaries[${ + summaries.length - 1 + }].decision="${lastRoundDecision}"`; + } + + return null; +} + +export function readReviewLedgerFile( + projectRoot: string, + changeName: string, + family: ReviewLedgerFamily, +): ArtifactReadResult<ReviewLedger> { + const path = reviewLedgerPath(projectRoot, changeName, family); + if (!existsSync(path)) return { kind: "absent" }; + let raw: string; + try { + raw = readFileSync(path, "utf8"); + } catch (err) { + return { + kind: "unreadable", + reason: err instanceof Error ? err.message : String(err), + }; + } + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch (err) { + return { + kind: "malformed", + reason: err instanceof Error ? err.message : String(err), + }; + } + const reason = validateReviewLedgerSchema(parsed); + if (reason !== null) { + return { kind: "malformed", reason }; + } + return { kind: "ok", value: parsed as ReviewLedger }; +} + +export function readDesignReviewLedger( + projectRoot: string, + changeName: string, +): ArtifactReadResult<ReviewLedger> { + return readReviewLedgerFile(projectRoot, changeName, "design"); +} + +export function readApplyReviewLedger( + projectRoot: string, + changeName: string, +): ArtifactReadResult<ReviewLedger> { + return readReviewLedgerFile(projectRoot, changeName, "apply"); +} + +// Re-export ledger types so downstream watcher code never reaches into +// orchestration-side helpers; only shape types from `contracts.ts` are used. +export type { LedgerRoundSummary, ReviewFinding, ReviewLedger }; + // --------------------------------------------------------------------------- // Task graph (per change) // --------------------------------------------------------------------------- diff --git a/src/lib/watch-renderer/index.ts b/src/lib/watch-renderer/index.ts index bc3b43b..03c6988 100644 --- a/src/lib/watch-renderer/index.ts +++ b/src/lib/watch-renderer/index.ts @@ -13,11 +13,16 @@ export { } from "./ansi.js"; export type { ApprovalSummaryView, + BuildDigestStateInput, BuildReviewViewInput, BundleTaskView, BundleView, + DigestFinding, + DigestSeverity, EventView, + LedgerDigest, ManualFixKind, + ReviewLayerState, ReviewRoundView, ReviewRoundVisibility, SectionState, @@ -27,11 +32,13 @@ export type { } from "./model.js"; export { buildApprovalSummary, + buildDigestState, buildEventsView, buildHeader, buildReviewView, buildTaskGraphView, deriveManualFixKind, + rankOpenFindings, terminalBannerFor, } from "./model.js"; export { renderFrame } from "./render.js"; diff --git a/src/lib/watch-renderer/model.ts b/src/lib/watch-renderer/model.ts index b47d2cc..7dbe348 100644 --- a/src/lib/watch-renderer/model.ts +++ b/src/lib/watch-renderer/model.ts @@ -3,9 +3,16 @@ // it into ANSI text. Separating the model keeps both halves unit-testable. import type { AutofixProgressSnapshot } from "../../types/autofix-progress.js"; -import type { RunState } from "../../types/contracts.js"; +import type { + ReviewFinding, + ReviewLedger, + RunState, +} from "../../types/contracts.js"; import type { RawObservationEvent } from "../observation-event-reader.js"; -import type { ArtifactReadResult } from "../specflow-watch/artifact-readers.js"; +import type { + ArtifactReadResult, + ReviewLedgerFamily, +} from "../specflow-watch/artifact-readers.js"; import type { Bundle, TaskStatus } from "../task-planner/index.js"; /** Per-section state tag: present / placeholder / warning. */ @@ -14,6 +21,54 @@ export type SectionState<T> = | { readonly kind: "placeholder"; readonly message: string } | { readonly kind: "warning"; readonly message: string }; +/** + * Per-layer state for the composite Review section. Adds a fourth `hidden` + * kind so non-review phases (e.g., `proposal_*`, `spec_*`) can suppress the + * digest layer entirely without colliding with `placeholder` (which is + * reserved for active review-family phases that have no ledger yet). + */ +export type ReviewLayerState<T> = + | { readonly kind: "ok"; readonly value: T } + | { readonly kind: "placeholder"; readonly message: string } + | { readonly kind: "warning"; readonly message: string } + | { readonly kind: "hidden" }; + +/** Discriminated state for the latest-round narrative summary. */ +export type SummaryState = + | { readonly kind: "available"; readonly text: string } + | { readonly kind: "absent" }; + +/** Compact, three-tier severity bucket displayed in the digest. */ +export type DigestSeverity = "HIGH" | "MEDIUM" | "LOW"; + +export interface DigestFinding { + readonly severity: DigestSeverity; + readonly title: string; + readonly id: string; +} + +/** Ledger-backed digest sub-model rendered below the snapshot progress lines. */ +export interface LedgerDigest { + readonly family: ReviewLedgerFamily; + /** Latest decision verbatim, or "(none)" when unavailable. */ + readonly decision: string; + readonly counts: { + readonly total: number; + readonly open: number; + readonly new_count: number; + readonly resolved: number; + }; + /** Severity counts over **open** findings only. `critical` aggregates into HIGH. */ + readonly openSeverity: { + readonly high: number; + readonly medium: number; + readonly low: number; + }; + readonly summaryState: SummaryState; + /** Up to three open findings ranked per the design's tie-break rule. */ + readonly topOpen: readonly DigestFinding[]; +} + export type ManualFixKind = "idle" | "design" | "apply"; export interface WatchModelHeader { @@ -90,6 +145,11 @@ export interface WatchModel { readonly header: WatchModelHeader; readonly terminal_banner: string | null; readonly review: SectionState<ReviewRoundView>; + /** + * Independent ledger-digest layer rendered below the snapshot progress lines + * inside the same Review section. `hidden` for non-review phases. + */ + readonly digest: ReviewLayerState<LedgerDigest>; readonly task_graph: SectionState<TaskGraphView>; readonly events: SectionState<readonly EventView[]>; readonly approval_summary: SectionState<ApprovalSummaryView>; @@ -424,3 +484,176 @@ export function buildApprovalSummary( } } } + +// --------------------------------------------------------------------------- +// Ledger digest model — derived exclusively from the latest persisted ledger +// state. The digest is self-sufficient; no external summary files are read. +// --------------------------------------------------------------------------- + +const SEVERITY_RANK: Record<string, number> = { + critical: 0, + high: 0, + medium: 1, + low: 2, +}; + +function severityRank(s: string | undefined): number { + if (s === undefined) return 0; // unknown → top tier + const key = s.toLowerCase(); + const rank = SEVERITY_RANK[key]; + return rank === undefined ? 0 : rank; +} + +function digestSeverityFor(s: string | undefined): DigestSeverity { + if (s === undefined) return "HIGH"; + const key = s.toLowerCase(); + if (key === "medium") return "MEDIUM"; + if (key === "low") return "LOW"; + // "critical", "high", and any unknown severity all display as HIGH per D6. + return "HIGH"; +} + +function isOpen(f: ReviewFinding): boolean { + const s = f.status; + return s === "open" || s === "new"; +} + +/** + * Rank open findings by severity (HIGH/CRITICAL > MEDIUM > LOW), then + * `latest_round` DESC, then finding `id` ASC. Pure — caller takes a slice. + */ +export function rankOpenFindings( + findings: readonly ReviewFinding[], +): readonly ReviewFinding[] { + return [...findings].filter(isOpen).sort((a, b) => { + const sa = severityRank(a.severity); + const sb = severityRank(b.severity); + if (sa !== sb) return sa - sb; + const ra = a.latest_round ?? 0; + const rb = b.latest_round ?? 0; + if (ra !== rb) return rb - ra; + const ia = a.id ?? ""; + const ib = b.id ?? ""; + return ia.localeCompare(ib); + }); +} + +/** + * Build the latest-round narrative state for the digest. + * + * The persisted `LedgerRoundSummary` does not carry free-form summary text + * (only counters and a decision token), and the watcher's read-only contract + * forbids reading any other artifact — `review-result*.json` was rejected by + * apply review as out-of-scope read-surface widening. With no compliant + * persisted source for narrative text, the digest renders no `Latest summary:` + * line. The spec explicitly allows this via "Latest round summary missing + * elides the line." When persisted narrative text becomes available in a + * future change (e.g. by extending `LedgerRoundSummary` with a free-form + * `summary` field), this function can return `{ kind: "available" }` + * instead. + */ +function buildSummaryState( + _latestRound: import("../../types/contracts.js").LedgerRoundSummary, +): SummaryState { + return { kind: "absent" }; +} + +function buildDigestFromLedger( + ledger: ReviewLedger, + family: ReviewLedgerFamily, +): LedgerDigest | null { + const summaries = ledger.round_summaries; + if (!Array.isArray(summaries) || summaries.length === 0) return null; + const last = summaries[summaries.length - 1]; + + const latestDecisionField = + typeof ledger.latest_decision === "string" && + ledger.latest_decision.length > 0 + ? ledger.latest_decision + : null; + const lastRoundDecisionField = + typeof last.decision === "string" && last.decision.length > 0 + ? last.decision + : null; + const decision = latestDecisionField ?? lastRoundDecisionField ?? "(none)"; + + const counts = { + total: last.total, + open: last.open, + new_count: last.new, + resolved: last.resolved, + }; + + const openFindings = ledger.findings.filter(isOpen); + let high = 0; + let medium = 0; + let low = 0; + for (const f of openFindings) { + const tier = digestSeverityFor(f.severity); + if (tier === "HIGH") high++; + else if (tier === "MEDIUM") medium++; + else low++; + } + + const ranked = rankOpenFindings(ledger.findings); + const topOpen: DigestFinding[] = ranked.slice(0, 3).map((f) => ({ + severity: digestSeverityFor(f.severity), + title: f.title ?? "", + id: f.id ?? "", + })); + + const summaryState = buildSummaryState(last); + + return { + family, + decision, + counts, + openSeverity: { high, medium, low }, + summaryState, + topOpen, + }; +} + +export interface BuildDigestStateInput { + readonly activeFamily: ReviewLedgerFamily | null; + readonly ledgerRead: ArtifactReadResult<ReviewLedger>; +} + +/** + * Build the digest layer state from the active family and the ledger reader + * result. Returns `hidden` when the phase is outside review families, + * `placeholder` when the ledger is absent or empty, `warning` for I/O or + * schema problems, and `ok` with a populated `LedgerDigest` otherwise. + * + * The digest is sourced exclusively from the ledger — no external summary + * files are consulted. The digest layer is independent from the snapshot + * layer — a snapshot placeholder/warning never suppresses a digest, and + * vice versa. + */ +export function buildDigestState( + input: BuildDigestStateInput, +): ReviewLayerState<LedgerDigest> { + const { activeFamily, ledgerRead } = input; + if (activeFamily === null) return { kind: "hidden" }; + switch (ledgerRead.kind) { + case "absent": + return { kind: "placeholder", message: "No review digest yet" }; + case "unreadable": + return { + kind: "warning", + message: `Review ledger unreadable: ${ledgerRead.reason}`, + }; + case "malformed": + return { + kind: "warning", + message: `Review ledger malformed: ${ledgerRead.reason}`, + }; + case "ok": { + const digest = buildDigestFromLedger(ledgerRead.value, activeFamily); + if (digest === null) { + return { kind: "placeholder", message: "No review digest yet" }; + } + return { kind: "ok", value: digest }; + } + } +} diff --git a/src/lib/watch-renderer/render.ts b/src/lib/watch-renderer/render.ts index 3c343d8..1946490 100644 --- a/src/lib/watch-renderer/render.ts +++ b/src/lib/watch-renderer/render.ts @@ -20,6 +20,8 @@ import { import type { BundleTaskView, BundleView, + LedgerDigest, + ReviewLayerState, SectionState, WatchModel, WatchModelHeader, @@ -174,6 +176,72 @@ function renderReviewSection( }); } +const NARROW_TERMINAL_THRESHOLD = 80; + +function ellipsizeForCols(text: string, cols: number): string { + if (cols <= 0) return ""; + if (visibleLen(text) <= cols) return text; + if (cols === 1) return "…"; + return `${truncateVisible(text, cols - 1)}…`; +} + +function applyNarrowTerminalRule( + lines: readonly string[], + cols: number, +): readonly string[] { + if (cols >= NARROW_TERMINAL_THRESHOLD) return lines; + return lines.map((line) => ellipsizeForCols(line, cols)); +} + +function renderDigestBody(d: LedgerDigest, cols: number): readonly string[] { + const lines: string[] = []; + lines.push(`Decision: ${d.decision}`); + lines.push( + `Findings: ${d.counts.total} total | ${d.counts.open} open | ${d.counts.new_count} new | ${d.counts.resolved} resolved`, + ); + lines.push( + `Severity: HIGH ${d.openSeverity.high} | MEDIUM ${d.openSeverity.medium} | LOW ${d.openSeverity.low}`, + ); + const summary = d.summaryState; + if (summary.kind === "available") { + lines.push(`Latest summary: ${summary.text}`); + } + // Narrow-terminal rule: drop the open-findings list and ellipsize the + // decision/counts/severity/summary lines. Wide terminals render in full. + if (cols < NARROW_TERMINAL_THRESHOLD) { + return applyNarrowTerminalRule(lines, cols); + } + if (d.topOpen.length > 0) { + lines.push("Open findings:"); + for (const f of d.topOpen) { + const sevColor = + f.severity === "HIGH" + ? FG_RED + : f.severity === "MEDIUM" + ? FG_YELLOW + : DIM; + lines.push(` ${color(f.severity, sevColor)} ${f.title}`); + } + } + return lines; +} + +function renderDigestSection( + state: ReviewLayerState<LedgerDigest>, + cols: number, +): readonly string[] { + switch (state.kind) { + case "hidden": + return []; + case "placeholder": + return [color(state.message, DIM)]; + case "warning": + return [color(`⚠ ${state.message}`, FG_RED)]; + case "ok": + return renderDigestBody(state.value, cols); + } +} + function renderTaskGraphSection( state: WatchModel["task_graph"], cols: number, @@ -302,9 +370,11 @@ export function renderFrame( ); } lines.push(padEndVisible("", c)); - lines.push( - ...renderSection("Review round", renderReviewSection(model.review, c), c), - ); + const reviewBody = renderReviewSection(model.review, c); + const digestBody = renderDigestSection(model.digest, c); + const combined = + digestBody.length === 0 ? reviewBody : [...reviewBody, ...digestBody]; + lines.push(...renderSection("Review round", combined, c)); lines.push(padEndVisible("", c)); lines.push( ...renderSection( diff --git a/src/tests/specflow-watch-integration.test.ts b/src/tests/specflow-watch-integration.test.ts index f42521d..fb3673d 100644 --- a/src/tests/specflow-watch-integration.test.ts +++ b/src/tests/specflow-watch-integration.test.ts @@ -7,12 +7,15 @@ import { phaseIsLiveReviewGate, readApprovalSummary, readAutofixSnapshotFile, + readReviewLedgerFile, readRunStateFile, readTaskGraphFile, selectActiveAutofixPhase, + selectActiveReviewLedger, } from "../lib/specflow-watch/artifact-readers.js"; import { buildApprovalSummary, + buildDigestState, buildEventsView, buildHeader, buildReviewView, @@ -200,6 +203,20 @@ function buildModelFor(root: string, run: RunState) { snapshot: reviewRead, manual_fix_kind: manualFixKind, }), + digest: buildDigestState({ + activeFamily: selectActiveReviewLedger(current.current_phase), + ledgerRead: + selectActiveReviewLedger(current.current_phase) !== null && + current.change_name + ? readReviewLedgerFile( + root, + current.change_name, + selectActiveReviewLedger(current.current_phase) as + | "design" + | "apply", + ) + : { kind: "absent" as const }, + }), task_graph: buildTaskGraphView( taskGraphRead.kind === "ok" ? { kind: "ok", value: { bundles: taskGraphRead.value.bundles } } @@ -425,6 +442,142 @@ test("integration: apply_draft shows only apply_review snapshot, never design_re } }); +function writeReviewLedger( + root: string, + changeName: string, + family: "design" | "apply", + overrides: Record<string, unknown> = {}, +): void { + const dir = join(root, "openspec/changes", changeName); + mkdirSync(dir, { recursive: true }); + const filename = + family === "design" ? "review-ledger-design.json" : "review-ledger.json"; + const ledger = { + feature_id: changeName, + phase: family, + current_round: 1, + status: "has_open_high", + max_finding_id: 0, + findings: [], + round_summaries: [ + { + round: 1, + total: 0, + open: 0, + new: 0, + resolved: 0, + overridden: 0, + by_severity: {}, + }, + ], + ...overrides, + }; + writeFileSync( + join(dir, filename), + `${JSON.stringify(ledger, null, 2)}\n`, + "utf8", + ); +} + +test("integration: digest renders from review-ledger-design.json during design phase", () => { + const root = makeTempDir("watch-int-digest-design-"); + try { + const run = seedRun(root, { + runId: "dgd-1", + changeName: "dgd", + currentPhase: "design_review", + status: "active", + }); + writeReviewLedger(root, "dgd", "design", { + latest_decision: "request_changes", + findings: [ + { + id: "R1-F01", + title: "auth boundary mismatch", + severity: "high", + status: "open", + origin_round: 1, + latest_round: 1, + }, + { + id: "R1-F02", + title: "retry path unclear", + severity: "medium", + status: "open", + origin_round: 1, + latest_round: 1, + }, + ], + round_summaries: [ + { + round: 1, + total: 2, + open: 2, + new: 2, + resolved: 0, + overridden: 0, + by_severity: { high: 1, medium: 1 }, + decision: "request_changes", + }, + ], + }); + const model = buildModelFor(root, run); + const frame = renderFrame(model, 120, 40) + .map((l) => stripAnsi(l)) + .join("\n"); + assert.match(frame, /Decision: request_changes/); + assert.match(frame, /Findings: 2 total \| 2 open \| 2 new \| 0 resolved/); + assert.match(frame, /Severity: HIGH 1 \| MEDIUM 1 \| LOW 0/); + assert.match(frame, /Open findings:/); + assert.match(frame, /HIGH\s+auth boundary mismatch/); + } finally { + removeTempDir(root); + } +}); + +test("integration: digest redraws when ledger updates on disk between frames", () => { + const root = makeTempDir("watch-int-digest-redraw-"); + try { + const run = seedRun(root, { + runId: "redraw-1", + changeName: "redraw", + currentPhase: "apply_review", + status: "active", + }); + // First render: no ledger on disk + const frame1 = renderFrame(buildModelFor(root, run), 120, 40) + .map((l) => stripAnsi(l)) + .join("\n"); + assert.match(frame1, /No review digest yet/); + + // Write the apply ledger + writeReviewLedger(root, "redraw", "apply", { + latest_decision: "approve", + round_summaries: [ + { + round: 1, + total: 0, + open: 0, + new: 0, + resolved: 0, + overridden: 0, + by_severity: {}, + decision: "approve", + }, + ], + }); + + // Second render reflects on-disk ledger + const frame2 = renderFrame(buildModelFor(root, run), 120, 40) + .map((l) => stripAnsi(l)) + .join("\n"); + assert.match(frame2, /Decision: approve/); + assert.doesNotMatch(frame2, /No review digest yet/); + } finally { + removeTempDir(root); + } +}); + test("integration: design_review phase renders only the design snapshot", () => { const root = makeTempDir("watch-int-selector-design-"); try { diff --git a/src/tests/specflow-watch-readers.test.ts b/src/tests/specflow-watch-readers.test.ts index 5ac8e26..f8ff968 100644 --- a/src/tests/specflow-watch-readers.test.ts +++ b/src/tests/specflow-watch-readers.test.ts @@ -10,13 +10,18 @@ import { import { approvalSummaryPath, autofixSnapshotPath, + readApplyReviewLedger, readApprovalSummary, readAutofixSnapshotFile, + readDesignReviewLedger, readRunStateFile, readTaskGraphFile, + reviewLedgerPath, runStatePath, selectActiveAutofixPhase, + selectActiveReviewLedger, taskGraphPath, + validateReviewLedgerSchema, } from "../lib/specflow-watch/artifact-readers.js"; import { resolveTrackedRun } from "../lib/specflow-watch/run-resolution.js"; import { parseRunJson, scanRuns } from "../lib/specflow-watch/run-scan.js"; @@ -459,3 +464,228 @@ test("readApprovalSummary: file exists but missing Status/What Changed → nulls removeTempDir(root); } }); + +// --------------------------------------------------------------------------- +// Review ledger readers + family selector +// --------------------------------------------------------------------------- + +function minimalLedger(overrides: Record<string, unknown> = {}): unknown { + return { + feature_id: "x", + phase: "design", + current_round: 1, + status: "all_resolved", + max_finding_id: 0, + findings: [], + round_summaries: [ + { + round: 1, + total: 0, + open: 0, + new: 0, + resolved: 0, + overridden: 0, + by_severity: {}, + }, + ], + ...overrides, + }; +} + +test("selectActiveReviewLedger: family mapping mirrors selectActiveAutofixPhase", () => { + // Design family → "design" + assert.equal(selectActiveReviewLedger("design_draft"), "design"); + assert.equal(selectActiveReviewLedger("design_review"), "design"); + assert.equal(selectActiveReviewLedger("design_ready"), "design"); + // Apply family → "apply" + assert.equal(selectActiveReviewLedger("apply_draft"), "apply"); + assert.equal(selectActiveReviewLedger("apply_review"), "apply"); + assert.equal(selectActiveReviewLedger("apply_ready"), "apply"); + assert.equal(selectActiveReviewLedger("approved"), "apply"); + // Outside review families → null + assert.equal(selectActiveReviewLedger("proposal_draft"), null); + assert.equal(selectActiveReviewLedger("spec_verify"), null); + assert.equal(selectActiveReviewLedger("terminal"), null); + + // Parity check: every phase that maps to a review-autofix family must also + // map to the corresponding ledger family, and vice versa. + const phases = [ + "design_draft", + "design_review", + "design_ready", + "apply_draft", + "apply_review", + "apply_ready", + "approved", + "proposal_draft", + "spec_verify", + "terminal", + ]; + for (const phase of phases) { + const autofix = selectActiveAutofixPhase(phase); + const ledger = selectActiveReviewLedger(phase); + if (autofix === "design_review") { + assert.equal(ledger, "design", `parity mismatch for ${phase}`); + } else if (autofix === "apply_review") { + assert.equal(ledger, "apply", `parity mismatch for ${phase}`); + } else { + assert.equal(ledger, null, `parity mismatch for ${phase}`); + } + } +}); + +test("reviewLedgerPath: design vs apply filenames", () => { + assert.ok( + reviewLedgerPath("/r", "x", "design").endsWith( + "openspec/changes/x/review-ledger-design.json", + ), + ); + assert.ok( + reviewLedgerPath("/r", "x", "apply").endsWith( + "openspec/changes/x/review-ledger.json", + ), + ); +}); + +test("readDesignReviewLedger / readApplyReviewLedger: absent / unreadable / malformed / ok", () => { + const root = makeTempDir("specflow-watch-ledger-"); + try { + // Absent + assert.equal(readDesignReviewLedger(root, "x").kind, "absent"); + assert.equal(readApplyReviewLedger(root, "x").kind, "absent"); + + // Malformed: not JSON + const designPath = reviewLedgerPath(root, "x", "design"); + mkdirSync(join(root, "openspec/changes/x"), { recursive: true }); + writeFileSync(designPath, "not json", "utf8"); + const malformed = readDesignReviewLedger(root, "x"); + assert.equal(malformed.kind, "malformed"); + + // Malformed: JSON but missing required fields + writeFileSync(designPath, JSON.stringify({ feature_id: "x" }), "utf8"); + const partial = readDesignReviewLedger(root, "x"); + assert.equal(partial.kind, "malformed"); + + // OK: full schema + writeFileSync(designPath, JSON.stringify(minimalLedger()), "utf8"); + const ok = readDesignReviewLedger(root, "x"); + assert.equal(ok.kind, "ok"); + if (ok.kind === "ok") { + assert.equal(ok.value.feature_id, "x"); + assert.equal(ok.value.round_summaries.length, 1); + } + + // Apply ledger uses the other filename + const applyPath = reviewLedgerPath(root, "x", "apply"); + writeFileSync( + applyPath, + JSON.stringify(minimalLedger({ phase: "apply" })), + "utf8", + ); + const okApply = readApplyReviewLedger(root, "x"); + assert.equal(okApply.kind, "ok"); + if (okApply.kind === "ok") { + assert.equal(okApply.value.phase, "apply"); + } + } finally { + removeTempDir(root); + } +}); + +test("validateReviewLedgerSchema: empty round_summaries is valid (renders as placeholder downstream)", () => { + const result = validateReviewLedgerSchema( + minimalLedger({ round_summaries: [] }), + ); + assert.equal(result, null); +}); + +test("validateReviewLedgerSchema: decision parity violation is malformed", () => { + const reason = validateReviewLedgerSchema( + minimalLedger({ + latest_decision: "approve", + round_summaries: [ + { + round: 1, + total: 0, + open: 0, + new: 0, + resolved: 0, + overridden: 0, + by_severity: {}, + decision: "request_changes", + }, + ], + }), + ); + assert.match(reason ?? "", /decision parity violation/); +}); + +test("validateReviewLedgerSchema: latest_decision matches last round summary decision is OK", () => { + const reason = validateReviewLedgerSchema( + minimalLedger({ + latest_decision: "approve", + round_summaries: [ + { + round: 1, + total: 0, + open: 0, + new: 0, + resolved: 0, + overridden: 0, + by_severity: {}, + decision: "approve", + }, + ], + }), + ); + assert.equal(reason, null); +}); + +test("validateReviewLedgerSchema: rejects findings whose latest_round is not a number", () => { + const reason = validateReviewLedgerSchema( + minimalLedger({ + findings: [ + { + id: "R1-F01", + title: "bad round", + severity: "high", + status: "open", + latest_round: "1", + }, + ], + }), + ); + assert.match(reason ?? "", /findings\[0\]\.latest_round/); +}); + +test("validateReviewLedgerSchema: rejects empty finding objects (R1-F03 regression)", () => { + const reason = validateReviewLedgerSchema(minimalLedger({ findings: [{}] })); + assert.ok(reason !== null, "empty finding should be rejected"); + assert.match(reason ?? "", /findings\[0\]\.id: missing or not a string/); +}); + +test("validateReviewLedgerSchema: rejects findings missing required fields (R1-F03 regression)", () => { + // Only status present — id, title, severity are missing. + const reason = validateReviewLedgerSchema( + minimalLedger({ findings: [{ status: "open" }] }), + ); + assert.ok(reason !== null, "finding with only status should be rejected"); + assert.match(reason ?? "", /findings\[0\]\.id: missing or not a string/); + + // id + status present — title and severity still missing. + const reason2 = validateReviewLedgerSchema( + minimalLedger({ findings: [{ id: "R1-F01", status: "open" }] }), + ); + assert.ok(reason2 !== null, "finding missing title should be rejected"); + assert.match(reason2 ?? "", /findings\[0\]\.title: missing or not a string/); + + // All four mandatory fields present — should pass. + const reason3 = validateReviewLedgerSchema( + minimalLedger({ + findings: [ + { id: "R1-F01", title: "ok", severity: "high", status: "open" }, + ], + }), + ); + assert.equal(reason3, null); +}); diff --git a/src/tests/watch-renderer.test.ts b/src/tests/watch-renderer.test.ts index f03008a..b08079d 100644 --- a/src/tests/watch-renderer.test.ts +++ b/src/tests/watch-renderer.test.ts @@ -6,12 +6,14 @@ import type { Bundle, TaskGraph } from "../lib/task-planner/index.js"; import { type BuildReviewViewInput, buildApprovalSummary, + buildDigestState, buildEventsView, buildHeader, buildReviewView, buildTaskGraphView, deriveManualFixKind, type ManualFixKind, + rankOpenFindings, renderFrame, stripAnsi, terminalBannerFor, @@ -19,7 +21,11 @@ import { type WatchModel, } from "../lib/watch-renderer/index.js"; import type { AutofixProgressSnapshot } from "../types/autofix-progress.js"; -import type { RunState } from "../types/contracts.js"; +import type { + ReviewFinding, + ReviewLedger, + RunState, +} from "../types/contracts.js"; function ok<T>(value: T): ArtifactReadResult<T> { return { kind: "ok", value }; @@ -83,6 +89,7 @@ function modelFor(parts: Partial<WatchModel> = {}): WatchModel { header: headerFor({ phase: "apply_draft" }), terminal_banner: null, review: buildReviewView(reviewInput()), + digest: { kind: "hidden" }, task_graph: buildTaskGraphView({ kind: "absent" }), events: buildEventsView([]), approval_summary: buildApprovalSummary({ kind: "absent" }), @@ -683,3 +690,525 @@ test("renderFrame: narrow terminal still produces bounded-width output", () => { ); } }); + +// --------------------------------------------------------------------------- +// Ledger digest model + renderer tests +// --------------------------------------------------------------------------- + +function ledger(parts: Partial<ReviewLedger> = {}): ReviewLedger { + return { + feature_id: "x", + phase: "design", + current_round: 1, + status: "has_open_high", + max_finding_id: 0, + findings: [], + round_summaries: [ + { + round: 1, + total: 0, + open: 0, + new: 0, + resolved: 0, + overridden: 0, + by_severity: {}, + }, + ], + ...parts, + } as ReviewLedger; +} + +function finding( + id: string, + severity: string, + status: string, + latest_round = 1, + title = "", +): ReviewFinding { + return { + id, + title: title || `finding ${id}`, + severity: severity as ReviewFinding["severity"], + status: status as ReviewFinding["status"], + latest_round, + origin_round: latest_round, + } as ReviewFinding; +} + +test("buildDigestState: hidden when activeFamily is null (non-review phase)", () => { + const state = buildDigestState({ + activeFamily: null, + ledgerRead: { kind: "absent" }, + }); + assert.equal(state.kind, "hidden"); +}); + +test("buildDigestState: placeholder when ledger is absent within review family", () => { + const state = buildDigestState({ + activeFamily: "design", + ledgerRead: { kind: "absent" }, + }); + assert.equal(state.kind, "placeholder"); + if (state.kind === "placeholder") { + assert.equal(state.message, "No review digest yet"); + } +}); + +test("buildDigestState: warning on unreadable ledger preserves reason", () => { + const state = buildDigestState({ + activeFamily: "apply", + ledgerRead: { kind: "unreadable", reason: "EACCES" }, + }); + assert.equal(state.kind, "warning"); + if (state.kind === "warning") { + assert.match(state.message, /Review ledger unreadable: EACCES/); + } +}); + +test("buildDigestState: warning on malformed ledger preserves reason", () => { + const state = buildDigestState({ + activeFamily: "design", + ledgerRead: { kind: "malformed", reason: "decision parity violation" }, + }); + assert.equal(state.kind, "warning"); + if (state.kind === "warning") { + assert.match(state.message, /Review ledger malformed: decision parity/); + } +}); + +test("buildDigestState: empty round_summaries renders as placeholder, not ok", () => { + const state = buildDigestState({ + activeFamily: "design", + ledgerRead: { kind: "ok", value: ledger({ round_summaries: [] }) }, + }); + assert.equal(state.kind, "placeholder"); +}); + +test("buildDigestState: populates digest from latest round_summary + open findings", () => { + const state = buildDigestState({ + activeFamily: "design", + ledgerRead: { + kind: "ok", + value: ledger({ + latest_decision: "request_changes", + findings: [ + finding("R1-F01", "high", "open", 1, "auth boundary mismatch"), + finding("R1-F02", "medium", "open", 1, "retry path unclear"), + finding("R1-F03", "low", "resolved", 1, "fixed style"), + ], + round_summaries: [ + { + round: 1, + total: 3, + open: 2, + new: 0, + resolved: 1, + overridden: 0, + by_severity: { high: 1, medium: 1, low: 1 }, + decision: "request_changes", + }, + ], + }), + }, + }); + assert.equal(state.kind, "ok"); + if (state.kind === "ok") { + assert.equal(state.value.decision, "request_changes"); + assert.equal(state.value.counts.total, 3); + assert.equal(state.value.counts.open, 2); + assert.equal(state.value.counts.resolved, 1); + assert.equal(state.value.openSeverity.high, 1); + assert.equal(state.value.openSeverity.medium, 1); + assert.equal(state.value.openSeverity.low, 0); + assert.equal(state.value.topOpen.length, 2); + assert.equal(state.value.topOpen[0].severity, "HIGH"); + assert.equal(state.value.topOpen[1].severity, "MEDIUM"); + } +}); + +test("buildDigestState: critical severity aggregates into HIGH counts and HIGH display", () => { + const state = buildDigestState({ + activeFamily: "apply", + ledgerRead: { + kind: "ok", + value: ledger({ + findings: [ + finding("R1-F01", "critical", "open", 1, "RCE"), + finding("R1-F02", "high", "open", 1, "auth"), + finding("R1-F03", "medium", "open", 1, "log noise"), + ], + round_summaries: [ + { + round: 1, + total: 3, + open: 3, + new: 0, + resolved: 0, + overridden: 0, + by_severity: { critical: 1, high: 1, medium: 1 }, + }, + ], + }), + }, + }); + assert.equal(state.kind, "ok"); + if (state.kind === "ok") { + assert.equal(state.value.openSeverity.high, 2); // critical + high + assert.equal(state.value.openSeverity.medium, 1); + // First entry should be the critical finding, displayed as HIGH. + assert.equal(state.value.topOpen[0].severity, "HIGH"); + assert.equal(state.value.topOpen[0].id, "R1-F01"); + } +}); + +test("buildDigestState: ledger.latest_decision preferred over round_summaries decision when both present and equal", () => { + const state = buildDigestState({ + activeFamily: "design", + ledgerRead: { + kind: "ok", + value: ledger({ + latest_decision: "approve", + round_summaries: [ + { + round: 1, + total: 0, + open: 0, + new: 0, + resolved: 0, + overridden: 0, + by_severity: {}, + decision: "approve", + }, + ], + }), + }, + }); + assert.equal(state.kind, "ok"); + if (state.kind === "ok") { + assert.equal(state.value.decision, "approve"); + } +}); + +test("buildDigestState: falls back to last round summary decision when latest_decision absent", () => { + const state = buildDigestState({ + activeFamily: "design", + ledgerRead: { + kind: "ok", + value: ledger({ + round_summaries: [ + { + round: 1, + total: 0, + open: 0, + new: 0, + resolved: 0, + overridden: 0, + by_severity: {}, + decision: "request_changes", + }, + ], + }), + }, + }); + assert.equal(state.kind, "ok"); + if (state.kind === "ok") { + assert.equal(state.value.decision, "request_changes"); + } +}); + +test("buildDigestState: decision (none) when both decision sources empty", () => { + const state = buildDigestState({ + activeFamily: "design", + ledgerRead: { kind: "ok", value: ledger() }, + }); + assert.equal(state.kind, "ok"); + if (state.kind === "ok") assert.equal(state.value.decision, "(none)"); +}); + +test("buildDigestState: summaryState is absent — no compliant persisted summary source exists today", () => { + // The persisted ledger does not carry free-form summary text and the + // watcher's read-only contract forbids widening to `review-result*.json`. + // Per spec ("Latest round summary missing elides the line") the digest + // SHALL omit the Latest summary line until a compliant source is added. + const state = buildDigestState({ + activeFamily: "design", + ledgerRead: { + kind: "ok", + value: ledger({ + round_summaries: [ + { + round: 3, + total: 5, + open: 2, + new: 1, + resolved: 3, + overridden: 0, + by_severity: { high: 1, medium: 1 }, + decision: "request_changes", + }, + ], + }), + }, + }); + assert.equal(state.kind, "ok"); + if (state.kind === "ok") { + assert.equal(state.value.summaryState.kind, "absent"); + } +}); + +test("rankOpenFindings: severity → latest_round DESC → id ASC", () => { + const findings = [ + finding("R3-F01", "high", "open", 3), + finding("R2-F03", "high", "open", 2), + finding("R3-F02", "high", "open", 3), + finding("R4-F01", "medium", "open", 4), + finding("R5-F02", "low", "open", 5), + ]; + const ranked = rankOpenFindings(findings); + assert.equal(ranked[0].id, "R3-F01"); + assert.equal(ranked[1].id, "R3-F02"); + assert.equal(ranked[2].id, "R2-F03"); + assert.equal(ranked[3].id, "R4-F01"); + assert.equal(ranked[4].id, "R5-F02"); +}); + +test("rankOpenFindings: critical ranks alongside high (rank 0), not below low", () => { + const findings = [ + finding("R1-F02", "low", "open", 1), + finding("R1-F01", "critical", "open", 1), + finding("R1-F03", "medium", "open", 1), + ]; + const ranked = rankOpenFindings(findings); + assert.equal(ranked[0].id, "R1-F01"); // critical first + assert.equal(ranked[1].id, "R1-F03"); // medium next + assert.equal(ranked[2].id, "R1-F02"); // low last +}); + +test("rankOpenFindings: filters out resolved/accepted_risk/ignored findings", () => { + const findings = [ + finding("R1-F01", "high", "open", 1), + finding("R1-F02", "high", "resolved", 1), + finding("R1-F03", "high", "accepted_risk", 1), + finding("R1-F04", "high", "new", 1), + ]; + const ranked = rankOpenFindings(findings); + assert.equal(ranked.length, 2); + const ids = ranked.map((f) => f.id); + assert.deepEqual(ids.sort(), ["R1-F01", "R1-F04"]); +}); + +// Renderer tests for the digest layer + +function digestModel( + parts: Partial<WatchModel> = {}, + digestParts: Partial<{ + activeFamily: "design" | "apply" | null; + ledgerRead: ArtifactReadResult<ReviewLedger>; + }> = {}, +): WatchModel { + return { + header: headerFor({ phase: "design_review" }), + terminal_banner: null, + review: buildReviewView(reviewInput()), + digest: buildDigestState({ + activeFamily: + "activeFamily" in digestParts + ? (digestParts.activeFamily as "design" | "apply" | null) + : "design", + ledgerRead: digestParts.ledgerRead ?? { kind: "absent" }, + }), + task_graph: buildTaskGraphView({ kind: "absent" }), + events: buildEventsView([]), + approval_summary: buildApprovalSummary({ kind: "absent" }), + ...parts, + }; +} + +test("renderFrame: digest placeholder appears below snapshot when ledger absent in review family", () => { + const model = digestModel( + {}, + { activeFamily: "design", ledgerRead: { kind: "absent" } }, + ); + const lines = renderFrame(model, 120, 40); + const plain = lines.map((l) => stripAnsi(l)).join("\n"); + assert.match(plain, /No active review/); + assert.match(plain, /No review digest yet/); +}); + +test("renderFrame: digest layer is fully suppressed for non-review phases", () => { + const model = digestModel( + { header: headerFor({ phase: "spec_draft" }) }, + { activeFamily: null, ledgerRead: { kind: "absent" } }, + ); + const lines = renderFrame(model, 120, 40); + const plain = lines.map((l) => stripAnsi(l)).join("\n"); + assert.doesNotMatch(plain, /No review digest yet/); + assert.doesNotMatch(plain, /Decision:/); +}); + +test("renderFrame: warning ledger renders inline below snapshot section", () => { + const model = digestModel( + {}, + { + activeFamily: "design", + ledgerRead: { kind: "malformed", reason: "round_summaries: missing" }, + }, + ); + const lines = renderFrame(model, 120, 40); + const plain = lines.map((l) => stripAnsi(l)).join("\n"); + assert.match(plain, /⚠ Review ledger malformed: round_summaries/); +}); + +test("renderFrame: populated digest renders Decision/Findings/Severity/summary/Open findings", () => { + const model = digestModel( + {}, + { + activeFamily: "design", + ledgerRead: { + kind: "ok", + value: ledger({ + latest_decision: "approve_with_findings", + findings: [ + finding("R1-F01", "high", "open", 1, "auth boundary mismatch"), + finding("R1-F02", "medium", "open", 1, "retry path unclear"), + finding("R1-F03", "medium", "open", 1, "missing test coverage"), + ], + round_summaries: [ + { + round: 1, + total: 8, + open: 3, + new: 2, + resolved: 5, + overridden: 0, + by_severity: { high: 1, medium: 2 }, + decision: "approve_with_findings", + }, + ], + }), + }, + }, + ); + const lines = renderFrame(model, 120, 40); + const plain = lines.map((l) => stripAnsi(l)).join("\n"); + assert.match(plain, /Decision: approve_with_findings/); + assert.match(plain, /Findings: 8 total \| 3 open \| 2 new \| 5 resolved/); + assert.match(plain, /Severity: HIGH 1 \| MEDIUM 2 \| LOW 0/); + // Per spec, the Latest summary line is omitted when no persisted narrative + // summary source is available; the persisted ledger has only counters and + // decision tokens. + assert.doesNotMatch(plain, /Latest summary:/); + assert.match(plain, /Open findings:/); + assert.match(plain, /HIGH\s+auth boundary mismatch/); + assert.match(plain, /MEDIUM\s+retry path unclear/); +}); + +test("renderFrame: Latest summary line is omitted (no persisted narrative source)", () => { + const model = digestModel( + {}, + { + activeFamily: "design", + ledgerRead: { + kind: "ok", + value: ledger({ + round_summaries: [ + { + round: 5, + total: 4, + open: 1, + new: 0, + resolved: 3, + overridden: 0, + by_severity: { high: 1 }, + decision: "request_changes", + }, + ], + }), + }, + }, + ); + const lines = renderFrame(model, 120, 40); + const plain = lines.map((l) => stripAnsi(l)).join("\n"); + assert.doesNotMatch(plain, /Latest summary:/); +}); + +test("renderFrame: narrow terminal (<80 cols) collapses Open findings list and ellipsizes other digest lines", () => { + const longTitle = "x".repeat(120); + const model = digestModel( + {}, + { + activeFamily: "design", + ledgerRead: { + kind: "ok", + value: ledger({ + findings: [finding("R1-F01", "high", "open", 1, longTitle)], + round_summaries: [ + { + round: 1, + total: 1, + open: 1, + new: 0, + resolved: 0, + overridden: 0, + by_severity: { high: 1 }, + decision: + "request_changes_with_a_very_long_decision_string_to_force_overflow", + }, + ], + }), + }, + }, + ); + const lines = renderFrame(model, 60, 40); + const plain = lines.map((l) => stripAnsi(l)); + const joined = plain.join("\n"); + assert.doesNotMatch(joined, /Open findings:/); + // Decision/Findings/Severity still present. + assert.match(joined, /Decision:/); + // All digest lines bounded to 60 cols; longest one is ellipsized. + const decisionLine = plain.find((l) => l.startsWith("Decision:")); + assert.ok(decisionLine !== undefined && decisionLine.length <= 60); +}); + +test("renderFrame: ledger-only (snapshot absent) still renders digest", () => { + const model = digestModel( + { + header: headerFor({ phase: "design_review" }), + review: buildReviewView( + reviewInput({ + phase_in_review_family: true, + phase_is_review_gate: true, + snapshot: { kind: "absent" }, + }), + ), + }, + { + activeFamily: "design", + ledgerRead: { + kind: "ok", + value: ledger({ + round_summaries: [ + { + round: 1, + total: 0, + open: 0, + new: 0, + resolved: 0, + overridden: 0, + by_severity: {}, + decision: "approve", + }, + ], + }), + }, + }, + ); + const lines = renderFrame(model, 120, 40); + const plain = lines.map((l) => stripAnsi(l)).join("\n"); + // snapshot absent → shows placeholder + assert.match(plain, /No active review/); + // Digest still appears with decision/counts/severity. Latest summary line + // is intentionally omitted — no compliant persisted narrative source. + assert.match(plain, /Decision: approve/); + assert.doesNotMatch(plain, /Latest summary:/); +});