From 487737add54459d1d7afd1ae94171f3cf32ad19b Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Tue, 25 Aug 2026 18:18:45 -0500 Subject: [PATCH 1/3] feat(ide): a skipped live lookup offers the stubbing affordance instead of a dead end (BACKLOG #236) The Steps view already surfaced a skipped `db_lookup`/`fhir_lookup` -- it rendered "live lookup - not evaluated in preview" on the row. That tells an author what did NOT happen and nothing about what to do, which is a dead end at exactly the moment they need a next step. ADR 0010 already specifies the next step, at :62: "a feed that uses it is previewed by stubbing its wrapper". The row's tooltip now names that mechanism. Nothing is mocked, no engine file changes, and the tracer's behaviour is untouched. THE ROW'S STATED HARD PART IS FALSE AND THIS DEPENDS ON THAT. #236 says lookup rows "do real I/O", so pin/mock "must be the default for lookup rows". They do not do I/O in a dry-run -- they RAISE. `classify_live_lookup` (dryrun_trace.py:281, called at :400) annotates the handler line and re-raises, so the disposition stays byte-identical. Because the failure is already structured and already reaches the IDE, the affordance is a tooltip rather than an engine feature. SCOPE, as ruled: the IDE affordance only. Engine mocking was declined by the owner, so ADR 0010's determinism decision stands untouched. The CLI stop condition is NOT in this item -- it is a separate unfiled row, and `dryrun_trace.py` is a sys.settrace OBSERVER, so stopping a handler mid-execution would change the execution semantics of the thing being observed. Nothing here goes near it. ALSO FIXED, BEYOND THE ITEM'S WORDING, AND DISCLOSED RATHER THAN FOLDED IN. `mergeLiveValues` folded each annotation's TEXT onto the row and dropped its `kind`, so a skipped-lookup row rendered inside the live-value span and inherited its tooltip: "Live value (redacted by default - synthetic samples only)". It is not a value and nothing was redacted -- an author hovering a skipped lookup was told the opposite of what happened. The kind now travels with the text. This is adjacent to the item rather than outside it: the affordance cannot be offered without knowing which annotation the row holds, so the mislabel and the feature are the same defect seen from two sides. The inline text keeps its existing warning glyph, which belongs to BACKLOG #1265's migration; the text added here carries none, and a test pins that. Verified: `npm run typecheck` clean; `npm run test:unit` 627 passing, 0 failing. 10 new tests, confirmed present in the run output by name rather than inferred from the total. Five mutants, all killed, each by a DISTINCT red set: drop the kind in `mergeLiveValues`; always render the value tooltip; let a value beat a warning in a mixed row; strip the mechanism from the tip; put a redaction claim back in it. Every mutant asserted a unique anchor, a changed file hash, and an unchanged NUL count -- `ide/src/stepsModel.ts` carries three deliberate NULs under `control_char_check.py`'s `NUL_IS_CONTENT_UNDER` allowance, and a mutation that disturbed them would be a different change wearing this one's clothes. Co-Authored-By: Claude Opus 5 --- ide/src/liveDebug.ts | 4 +- ide/src/stepsModel.ts | 33 ++++++++- ide/src/test/suite/steps.test.ts | 123 +++++++++++++++++++++++++++++++ 3 files changed, 158 insertions(+), 2 deletions(-) diff --git a/ide/src/liveDebug.ts b/ide/src/liveDebug.ts index 7483628d8..f36ea7146 100644 --- a/ide/src/liveDebug.ts +++ b/ide/src/liveDebug.ts @@ -350,7 +350,9 @@ export function inlineValuesFor(invocations: TraceInvocation[], reveal: boolean) out.push({ line: line - 1, after: WARNING_TEXT, - hover: `\`${call}\` is a live, read-only lookup — not evaluated in this offline preview.`, + hover: + `\`${call}\` is a live, read-only lookup — not evaluated in this offline preview.\n\n` + + "Preview the feed by stubbing this call's wrapper function (ADR 0010).", kind: "warning", }); } diff --git a/ide/src/stepsModel.ts b/ide/src/stepsModel.ts index 128a19f1b..5552315a7 100644 --- a/ide/src/stepsModel.ts +++ b/ide/src/stepsModel.ts @@ -232,6 +232,13 @@ export interface RowViewModel { editableParams?: string[]; code?: string; // verbatim source slice — code rows only (the degradation-ladder passthrough) liveValue?: string; // redacted-by-default #92 annotation (see mergeLiveValues); undefined = none + /** + * Which kind of annotation `liveValue` holds. `mergeLiveValues` folds the inline values' TEXT into one + * string and used to drop their `kind`, so a skipped-lookup row rendered inside the live-value tooltip + * and told the author it was showing a redacted live value -- the opposite of what happened (BACKLOG + * #236). Carried so the renderer can offer the right affordance instead. + */ + liveValueKind?: "value" | "warning"; /** * The PROJECTION-TIME source text of this row's [lineStart, lineEnd] range — the row exactly as the user * saw it when the lens projected the inputs (engine newline model: split on `\r\n`|`\r`|`\n`, joined by @@ -654,6 +661,25 @@ export interface LiveInlineValue { kind: "value" | "warning"; } +/** + * The tooltip on a row whose annotation is a SKIPPED LIVE LOOKUP (BACKLOG #236). + * + * A `db_lookup`/`fhir_lookup` does not do I/O in a dry-run -- it RAISES, and the tracer records a + * `live_lookup_skipped` annotation and re-raises so the disposition is byte-identical + * (`dryrun_trace.classify_live_lookup`). The row's old tooltip said "Live value (redacted by default)", + * which is wrong twice over: it is not a value and nothing was redacted. + * + * So the tooltip names the mechanism ADR 0010 already specifies instead of restating the failure -- + * `ADR 0010`: "a feed that uses it is previewed by stubbing its wrapper". That is the supported preview + * path, and pointing at it is the whole of this item: the engine is unchanged and nothing is mocked. + */ +export const LIVE_LOOKUP_TIP = + "Live read-only lookup: a dry-run cannot run it, so this row was not evaluated. " + + "Preview the feed by stubbing this call's wrapper function (ADR 0010)."; + +/** The tooltip on a row showing a real captured value. Unchanged wording (BACKLOG #92). */ +export const LIVE_VALUE_TIP = "Live value (redacted by default — synthetic samples only)"; + /** The redacted annotation shown by default (matches liveDebug's VALUE_MARKER + REVEAL_PLACEHOLDER). */ export const REDACTED_LIVE_VALUE = "▸ ⋯"; @@ -672,6 +698,9 @@ export function mergeLiveValues(rows: RowViewModel[], inline: LiveInlineValue[]) ); if (hits.length > 0) { row.liveValue = hits.map((h) => h.after).join(" · "); + // A warning WINS over a value. `traceRowValues` already suppresses values on a warned line, so a + // warning hit means this row's annotation is the skipped-lookup notice, not a reading. + row.liveValueKind = hits.some((h) => h.kind === "warning") ? "warning" : "value"; } } return rows; @@ -2624,8 +2653,10 @@ export function renderRowHtml( const indent = `style="margin-left:${row.nesting * INDENT_PX}px"`; const badge = row.badge ? `${escapeHtml(row.badge)}` : ""; const subtitle = row.subtitle ? `${escapeHtml(row.subtitle)}` : ""; + const liveTip = row.liveValueKind === "warning" ? LIVE_LOOKUP_TIP : LIVE_VALUE_TIP; + const liveClass = row.liveValueKind === "warning" ? "live warn" : "live"; const live = row.liveValue - ? `${escapeHtml(row.liveValue)}` + ? `${escapeHtml(row.liveValue)}` : ""; // A field is editable only when the handler name is known (the write path); read-only callers pass "". const editable = new Set(handlerName ? (row.editableParams ?? []) : []); diff --git a/ide/src/test/suite/steps.test.ts b/ide/src/test/suite/steps.test.ts index fd9a56ecb..e4101d665 100644 --- a/ide/src/test/suite/steps.test.ts +++ b/ide/src/test/suite/steps.test.ts @@ -5,6 +5,8 @@ import * as fs from "fs"; import * as path from "path"; import { + LIVE_LOOKUP_TIP, + LIVE_VALUE_TIP, REDACTED_LIVE_VALUE, buildHandlerViewModels, buildLensTraceArgs, @@ -318,6 +320,10 @@ suite("stepsModel — buildLensTraceArgs never requests PHI (the lens's redacted }); suite("stepsModel — traceRowValues folds a traced dry-run onto rows (redacted by default)", () => { + const lookupRow = (): RowViewModel[] => [ + { index: 0, kind: "action", nesting: 0, lineStart: 4, lineEnd: 4, title: "Lookup", params: [] }, + ]; + test("redacted by default (reveal off): each executed line is the ▸ ⋯ placeholder, no real value", () => { const off = traceRowValues([PRODUCING], false); assert.strictEqual(off.length, 2); @@ -387,6 +393,123 @@ suite("stepsModel — traceRowValues folds a traced dry-run onto rows (redacted assert.ok(warn?.after.includes("live lookup"), warn?.after); }); + test("a skipped lookup carries its KIND to the row, not just its text (BACKLOG #236)", () => { + // The affordance depends on the row knowing WHICH annotation it holds. `mergeLiveValues` used to + // fold only the text, so a skipped-lookup row was indistinguishable from a value row downstream. + const skipped = traceRowValues( + [inv({ annotations: [{ line: 4, kind: "live_lookup_skipped", call: "db_lookup" }] })], + false, + ); + const rows = lookupRow(); + mergeLiveValues(rows, skipped); + assert.strictEqual(rows[0].liveValueKind, "warning", "the row must know it holds a warning"); + }); + + test("a row holding real captured values is still marked a value", () => { + const rows = lookupRow(); + mergeLiveValues(rows, [{ line: 3, after: REDACTED_LIVE_VALUE, kind: "value" }]); + assert.strictEqual(rows[0].liveValueKind, "value"); + }); + + test("a warning WINS over a value landing in the same row", () => { + // `traceRowValues` already suppresses values on a warned LINE, but a multi-line row can hold both + // a warned line and a producing one. The row's affordance must follow the warning. + const rows = lookupRow(); + mergeLiveValues(rows, [ + { line: 3, after: REDACTED_LIVE_VALUE, kind: "value" }, + { line: 3, after: "not evaluated", kind: "warning" }, + ]); + assert.strictEqual(rows[0].liveValueKind, "warning"); + }); + + test("a row with no annotation gets no kind at all", () => { + const rows = lookupRow(); + mergeLiveValues(rows, []); + assert.strictEqual(rows[0].liveValueKind, undefined); + assert.strictEqual(rows[0].liveValue, undefined); + }); +}); + +// -------------------------------------------------------------------------------------------------- +// BACKLOG #236: the lookup row's tooltip must OFFER THE SUPPORTED PREVIEW PATH rather than restate the +// failure. A db_lookup/fhir_lookup does not do I/O in a dry-run -- it RAISES, and the tracer records a +// `live_lookup_skipped` annotation and re-raises (dryrun_trace.classify_live_lookup). ADR 0010 already +// names the mechanism: "a feed that uses it is previewed by stubbing its wrapper". Nothing is mocked and +// no engine file changes; the IDE just stops pointing at a dead end. +// -------------------------------------------------------------------------------------------------- + +suite("stepsModel — a skipped live lookup offers the stubbing affordance (BACKLOG #236)", () => { + const warnRow = (): RowViewModel => ({ + index: 0, + kind: "action", + nesting: 0, + lineStart: 4, + lineEnd: 4, + title: "Lookup", + params: [], + liveValue: "not evaluated", + liveValueKind: "warning", + }); + + test("the warning row's tooltip names the mechanism instead of restating the failure", () => { + const html = renderRowHtml(warnRow()); + assert.ok(html.includes("stubbing"), `expected the stubbing affordance, got: ${html}`); + assert.ok(html.includes("ADR 0010"), "the tooltip must cite where the mechanism is specified"); + }); + + test("THE MISLABEL: a skipped lookup no longer claims to be a redacted live value", () => { + // It is not a value and nothing was redacted. Telling an author the opposite of what happened is + // the defect this fixes; it was reached because mergeLiveValues dropped the kind. + const html = renderRowHtml(warnRow()); + assert.ok(!html.includes(LIVE_VALUE_TIP), "the value tooltip must not appear on a warning row"); + assert.ok(!html.includes("redacted"), `no redaction claim on a skipped lookup: ${html}`); + }); + + test("a real value row's tooltip is UNCHANGED (BACKLOG #92 wording preserved)", () => { + const row = { ...warnRow(), liveValue: REDACTED_LIVE_VALUE, liveValueKind: "value" as const }; + const html = renderRowHtml(row); + assert.ok(html.includes(LIVE_VALUE_TIP), "the existing value tooltip must survive verbatim"); + assert.ok(!html.includes(LIVE_LOOKUP_TIP), "a value row must not offer the lookup affordance"); + }); + + test("a row with no annotation renders no tooltip span at all", () => { + const row = { ...warnRow(), liveValue: undefined, liveValueKind: undefined }; + const html = renderRowHtml(row); + assert.ok(!html.includes(LIVE_LOOKUP_TIP) && !html.includes(LIVE_VALUE_TIP), html); + }); + + test("END TO END: a live_lookup_skipped annotation reaches the rendered row as the affordance", () => { + // The whole chain the author actually meets: tracer annotation -> traceRowValues -> mergeLiveValues + // -> renderRowHtml. Each step is covered above; this asserts they compose. + const skipped = traceRowValues( + [inv({ annotations: [{ line: 4, kind: "live_lookup_skipped", call: "db_lookup" }] })], + false, + ); + const rows = [ + { + index: 0, + kind: "action" as const, + nesting: 0, + lineStart: 4, + lineEnd: 4, + title: "Lookup", + params: [], + }, + ]; + mergeLiveValues(rows, skipped); + const html = renderRowHtml(rows[0]); + assert.ok(html.includes("stubbing"), `affordance missing end to end: ${html}`); + assert.ok(!html.includes("redacted"), `mislabel survived end to end: ${html}`); + }); + + test("the affordance text carries no glyph (CLAUDE.md section 11)", () => { + // The row's inline text keeps its pre-existing warning glyph, which belongs to BACKLOG #1265's + // migration. The text THIS item adds must not introduce another. + assert.ok(!/[\u2700-\u27bf\u2b00-\u2bff\u26a0\u2705\u274c\ufe0f]|[\ud800-\udbff]/.test(LIVE_LOOKUP_TIP), LIVE_LOOKUP_TIP); + }); +}); + +suite("stepsModel — traceRowValues, continued", () => { test("across traced messages, the newest invocation's value wins for a shared line", () => { const first = inv({ events: [{ line: 3, event: "line", assigned: { x: "AAA" } }] }); const second = inv({ events: [{ line: 3, event: "line", assigned: { x: "BBB" } }] }); From fda6568d6a65fca3b80d48a3f006e3f5a9e8d0fc Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Tue, 25 Aug 2026 18:29:15 -0500 Subject: [PATCH 2/3] backlog: #236 progress note -- the afterthought wrinkle is fixed (PR #605), the feature is not Records that PR #605 fixed the specific dead-end the Filed paragraph called out in passing (a skipped lookup's tooltip pointing nowhere useful) but built none of what the item's title and re-score describe: no stop condition, no state dump, no pin mechanism, no step-scoped run. Co-Authored-By: Claude Sonnet 5 --- docs/BACKLOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 61f77f17b..73024e08c 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -2893,6 +2893,16 @@ def route_demo_oru(msg): > 🔢 **Re-scored 2026-08-20 -> P2.** Value **5/10** · Difficulty **4/10** · _fill-in_. None of the step-scoped run symbols exist in ide/src, so the feature is unstarted, but the substrate it needs is shipped: the traced dry-run, the line-containment fold and the sample picker are all present. Value is capped at parity with a clean workaround, since running the whole handler in the Test Bench already works; difficulty sits at 4 because the stop condition and state dump ride an existing path while the pin mechanism, the redaction reuse and the IDE surface cross the CLI-to-extension seam. _(was 5/10 · 4/10.)_ > +> **PARTIAL 2026-08-25 -- THE "NOT AS AN AFTERTHOUGHT" WRINKLE IS FIXED (PR #605). THE STEP-SCOPED +> FEATURE ITSELF IS STILL UNSTARTED.** A skipped `db_lookup`/`fhir_lookup` used to render "live +> lookup - not evaluated in preview" and dead-end there; it now points at ADR 0010:62's supported +> preview path (stub the call's wrapper) instead. Also fixed: `mergeLiveValues` had dropped the +> annotation's kind, so a skipped lookup inherited the live-value tooltip "Live value (redacted by +> default)" -- it is not a value and nothing was redacted. **None of the feature this item names +> shipped:** no stop condition, no state dump, no pin mechanism, no "test up to row N" surface. +> None of the step-scoped run symbols this row's own re-score names as absent from `ide/src` exist +> yet. This PR fixes the UX dead-end the Filed paragraph called out in passing, not the item. +> > **Filed 2026-07-30 — not started.** Largely a stop condition + state dump on ADR 0072's traced dry-run; lookup rows must mock by default, not as an afterthought. > Verdict: build > Closing-act: code From 889c781ccde89011fdb5fb8a04d22caf3284a943 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Tue, 25 Aug 2026 18:32:56 -0500 Subject: [PATCH 3/3] backlog: #236 -- record the owner ruling and the stop-condition design constraint Builder2 relayed the owner's ruling via the Liaison: mocking-by-default declined even as a filed ADR 0010 amendment, closing #236 outright declined. Both confirm PARTIAL is the correct banner, not an oversight. Also records why the stop condition is unfiled rather than unstarted: it changes sys.settrace observation semantics rather than adding a flag, per the Dispatcher's split. Co-Authored-By: Claude Sonnet 5 --- docs/BACKLOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 73024e08c..aa1bc03d7 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -2902,6 +2902,14 @@ def route_demo_oru(msg): > shipped:** no stop condition, no state dump, no pin mechanism, no "test up to row N" surface. > None of the step-scoped run symbols this row's own re-score names as absent from `ide/src` exist > yet. This PR fixes the UX dead-end the Filed paragraph called out in passing, not the item. +> **Confirmed by the owner via the Liaison: mocking-by-default is DECLINED even as a filed ADR 0010 +> amendment, and closing this item OUTRIGHT is DECLINED.** Both records agree -- the ledger stayed +> open through this PR by design, not by oversight. +> **The stop condition is a design question, not a to-do -- do not add it to this item casually.** +> `dryrun_trace.py` implements the traced dry-run as a `sys.settrace` observer; stopping a handler +> mid-execution means raising out of the trace function, which changes the execution semantics of +> the thing being observed. It is not a flag to add. The Dispatcher split it out as its own unfiled +> piece for exactly this reason. > > **Filed 2026-07-30 — not started.** Largely a stop condition + state dump on ADR 0072's traced dry-run; lookup rows must mock by default, not as an afterthought. > Verdict: build