Skip to content

Commit 883367b

Browse files
alex-strukclaude
andcommitted
fix(workflow-builder): never-cached copy must not tell built-in authors to edit a script (D-18a)
Found walking 9.5. document.updateStatus is nonCacheable in the catalog, so my D-12 copy told its author to 'Tag it @deterministic true' — there is no script and no tag. Every nonCacheable built-in hits it: azureOcr.submit, document.storeRejection, every benchmark writer. describeNoOutput now splits on isDynamicNode: the dynamic-node text keeps the actionable advice, a built-in gets copy about the activity itself. Verified live on two nodes; test falsified against the fix removed. Also records the Part 9 walk it came out of. Azure OCR works — the standard-OCR chain finishes in under 12s on the sample invoice — which unblocked 9.4a, 9.5a and 9.10 (all pass) and the verified half of 9.5/9.5b/9.5c/9.10a. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 4c873b6 commit 883367b

6 files changed

Lines changed: 108 additions & 11 deletions

File tree

apps/frontend/src/features/workflow-builder/canvas/WorkflowEditorCanvas.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -829,6 +829,7 @@ const ActivityNodeRenderer = memo(
829829
nodeId={id}
830830
outputs={data.previewOutputs}
831831
neverCached={neverCached}
832+
isDynamicNode={isDynamic}
832833
/>
833834
</div>
834835
);

apps/frontend/src/features/workflow-builder/preview/PreviewWidget.tsx

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,12 @@ export interface PreviewWidgetProps {
110110
* cannot work.
111111
*/
112112
neverCached?: boolean;
113+
/**
114+
* D-18a — true only for `dyn.*` nodes. Splits the never-cached copy: a
115+
* dynamic node's author can tag the script `@deterministic true`; the author
116+
* of a built-in `nonCacheable` activity cannot, and must not be told to.
117+
*/
118+
isDynamicNode?: boolean;
113119
}
114120

115121
/**
@@ -129,6 +135,7 @@ export function PreviewWidget({
129135
nodeStatus,
130136
producesOutput = true,
131137
neverCached = false,
138+
isDynamicNode = false,
132139
}: PreviewWidgetProps): ReactNode {
133140
const { data, isLoading, error } = useActivityOutputPreview(
134141
workflowId,
@@ -181,7 +188,7 @@ export function PreviewWidget({
181188
hasActiveRun: hasRun,
182189
neverCached,
183190
});
184-
const copy = describeNoOutput(reason);
191+
const copy = describeNoOutput(reason, { isDynamicNode });
185192

186193
// Eviction is the ONE reason with a working recovery: the node genuinely
187194
// produced output and re-running repopulates the row. Every other reason
@@ -289,6 +296,12 @@ export interface NodePreviewOverlayProps {
289296
* cannot work.
290297
*/
291298
neverCached?: boolean;
299+
/**
300+
* D-18a — true only for `dyn.*` nodes. Splits the never-cached copy: a
301+
* dynamic node's author can tag the script `@deterministic true`; the author
302+
* of a built-in `nonCacheable` activity cannot, and must not be told to.
303+
*/
304+
isDynamicNode?: boolean;
292305
}
293306

294307
/**
@@ -303,6 +316,7 @@ export function NodePreviewOverlay({
303316
outputs,
304317
producesOutput = true,
305318
neverCached = false,
319+
isDynamicNode = false,
306320
}: NodePreviewOverlayProps): ReactNode {
307321
const ctx = useOptionalRunState();
308322
if (!ctx) {
@@ -336,6 +350,7 @@ export function NodePreviewOverlay({
336350
nodeStatus={ctx.nodeStatuses[nodeId]?.status}
337351
producesOutput={producesOutput}
338352
neverCached={neverCached}
353+
isDynamicNode={isDynamicNode}
339354
/>
340355
);
341356
}

apps/frontend/src/features/workflow-builder/preview/no-output-state.test.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,29 @@ describe("noOutputReasonForNode — D-12 never-cached vs evicted", () => {
158158
});
159159

160160
it("offers no Re-run for `not-cached`, and says why rather than blaming an eviction", () => {
161-
const copy = describeNoOutput("not-cached");
161+
const copy = describeNoOutput("not-cached", { isDynamicNode: true });
162162
expect(copy.offersRerun).toBe(false);
163163
expect(copy.message).toMatch(/non-deterministic/i);
164164
expect(copy.message).not.toMatch(/evict/i);
165165
});
166+
167+
// D-18a — found by walking 9.5 on the standard-OCR workflow, where
168+
// `document.updateStatus` (a catalog-level `nonCacheable` activity, no
169+
// script anywhere) told the author to "Tag it `@deterministic true`". Every
170+
// `nonCacheable` built-in hits this: azureOcr.submit, document.storeRejection,
171+
// every benchmark writer.
172+
it("does NOT tell a built-in activity's author to edit a script they do not have", () => {
173+
const builtIn = describeNoOutput("not-cached");
174+
expect(builtIn.offersRerun).toBe(false);
175+
expect(builtIn.message).not.toMatch(/@deterministic/i);
176+
expect(builtIn.message).not.toMatch(/\bscript\b/i);
177+
expect(builtIn.message).not.toMatch(/evict/i);
178+
// Still says what happened and why there is nothing to show.
179+
expect(builtIn.message).toMatch(/never caches/i);
180+
181+
// The dynamic-node copy keeps the actionable advice — it is actionable there.
182+
expect(
183+
describeNoOutput("not-cached", { isDynamicNode: true }).message,
184+
).toMatch(/@deterministic/i);
185+
});
166186
});

apps/frontend/src/features/workflow-builder/preview/no-output-state.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,19 @@ export interface NoOutputCopy {
104104
/**
105105
* The copy + affordances for each reason. Exhaustive over `NoOutputReason`.
106106
*/
107-
export function describeNoOutput(reason: NoOutputReason): NoOutputCopy {
107+
export function describeNoOutput(
108+
reason: NoOutputReason,
109+
options?: {
110+
/**
111+
* D-18a — only a dynamic node's author can change whether its output is
112+
* cached (by tagging the script `@deterministic true`). A built-in
113+
* activity is `nonCacheable` by catalog design — `document.updateStatus`,
114+
* `azureOcr.submit`, every `benchmark.*` writer — and telling that author
115+
* to edit a script they do not have is an instruction they cannot follow.
116+
*/
117+
isDynamicNode?: boolean;
118+
},
119+
): NoOutputCopy {
108120
switch (reason) {
109121
case "no-run":
110122
return {
@@ -165,7 +177,9 @@ export function describeNoOutput(reason: NoOutputReason): NoOutputCopy {
165177
return {
166178
reason,
167179
message:
168-
"This step ran, but its output isn't cached: the script is marked non-deterministic, so it re-executes every run instead of being stored. Tag it `@deterministic true` to make its output previewable.",
180+
options?.isDynamicNode === true
181+
? "This step ran, but its output isn't cached: the script is marked non-deterministic, so it re-executes every run instead of being stored. Tag it `@deterministic true` to make its output previewable."
182+
: "This step ran, but this activity never caches its output — it re-executes on every run instead of being stored, so there's nothing here to preview.",
169183
offersRerun: false,
170184
tone: "notable",
171185
};

docs-md/workflows/MANUAL_TEST_PLAN.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -466,13 +466,13 @@ Requires Temporal server + **worker** + visibility store + `activity_output_cach
466466
- [x] **9.3 Upload & Try.** On a saved source.upload workflow → the source node’s **Upload & Try** → drop a PDF. **Pass:** file commits to blob, a run starts, source node’s DocumentPreview shows the doc, canvas animates.
467467
- [x] **9.4 Status badges + active edges.** Watch a Try. **Pass:** per-node badge progresses pending→running→succeeded/failed/skipped on a ~1.5s poll (pauses on tab blur); active edge animates blue while source running/target pending.
468468
- API: `GET /api/workflows/<WF>/runs/<RUN>/node-statuses`.
469-
- [ ] **9.4a The path the run took (G-014).** Use a switch-heavy workflow (`multi-page-report-workflow`). Watch a Try, then let it finish. **Pass:** while running, the already-walked hops render in the lighter **taken-path** blue *behind* the animated in-flight edge (both cues visible at once — the animation is not replaced by the trail); once the run finishes nothing animates but the **full path it took stays drawn**. On the switch node, exactly ONE outgoing edge is on the path — the branch that was not chosen stays in its resting style. Repeat via **9.9 Replay** on a finished run: the path is still drawn, even though nothing is running. *(Before G-014 an edge was only marked while its source was CURRENTLY running, so a replay showed no path at all.)*
469+
- [x] **9.4a The path the run took (G-014).** Use a switch-heavy workflow (`multi-page-report-workflow`). Watch a Try, then let it finish. **Pass:** while running, the already-walked hops render in the lighter **taken-path** blue *behind* the animated in-flight edge (both cues visible at once — the animation is not replaced by the trail); once the run finishes nothing animates but the **full path it took stays drawn**. On the switch node, exactly ONE outgoing edge is on the path — the branch that was not chosen stays in its resting style. Repeat via **9.9 Replay** on a finished run: the path is still drawn, even though nothing is running. *(Before G-014 an edge was only marked while its source was CURRENTLY running, so a replay showed no path at all.)*
470470
- API: `GET /api/workflows/<WF>/runs/<RUN>/node-statuses` → a switch / humanGate-fallback / errorPolicy-fallback node carries `selectedEdgeId`; every other node omits it (meaning all outgoing normal edges were taken).
471-
- [ ] **9.5 Preview widgets.** **Pass:** Document → thumbnail strip; Segment[] → polygon overlays; OcrResult → K/V table + “View raw”; Classification → label pill + confidence bar. A kind with **no** dedicated widget (`ValidationResult`, `Reference`, bare `Artifact`, `Document[]`, scalar `Segment`, …) must render the **generic view**: a dimmed caption naming the kind (“ValidationResult — no dedicated preview, showing the raw value”) above a JSON snippet with **View raw**. **No node may render an empty card.** (G-011)
471+
- [ ] **9.5 Preview widgets.** *(generic-view + no-empty-card clauses verified 2026-07-27; the four kind-specific widgets still need a graph that produces them)* **Pass:** Document → thumbnail strip; Segment[] → polygon overlays; OcrResult → K/V table + “View raw”; Classification → label pill + confidence bar. A kind with **no** dedicated widget (`ValidationResult`, `Reference`, bare `Artifact`, `Document[]`, scalar `Segment`, …) must render the **generic view**: a dimmed caption naming the kind (“ValidationResult — no dedicated preview, showing the raw value”) above a JSON snippet with **View raw**. **No node may render an empty card.** (G-011)
472472
- API: `GET /api/workflows/<WF>/preview-cache?nodeId=<NODE>`.
473-
- [ ] **9.5a Multi-output preview.** Use a node with more than one output port (`ocr.spellcheck``correctedResult` / `corrections` / `metadata`; or `document.classify`, `azureOcr.poll`). After a Try, look at that node’s preview. **Pass:** a row of small port chips appears above the preview, labelled with the catalog port labels; the first port is selected and its value is shown; clicking another chip switches the pane to **that port’s** value, styled by that port’s own kind. A single-output node shows **no** chip row (unchanged). (G-011)
474-
- [ ] **9.5b Bound-but-empty and unbound outputs.** On a node whose output binding points at a ctx key the run never wrote: **Pass:** “No value was recorded for this output (expected `<Kind>`).” On a node with no output binding at all: **Pass:** “This step’s output isn’t bound to a workflow value yet, so there’s nothing to read.” Neither may be a blank card. (G-011)
475-
- [ ] **9.5c OCR previews show values, not a blob pointer.** On an OCR step (`azureOcr.extract`, `mistral-ocr.process`, any `ocr.*` correction). **Pass:** the K/V table shows the **payload’s** keys (e.g. `fileName`, `status`, `extractedText`, `pages`, `documents`) — **not** `blobPath` / `storage` / `byteLength`. For a custom-model run, drilling into `documents → fields` via **View raw** shows the extracted field values. If the payload was bounded, a dimmed line reads “Truncated preview — pages: showing the first 5 of N items; …”, naming every omission. If the blob is gone (delete it from MinIO and re-open the preview), **Pass:** the pointer is shown **with** “The full OCR payload is no longer in storage — showing the reference only.” — never a bare blob key with no explanation. (G-022)
473+
- [x] **9.5a Multi-output preview.** Use a node with more than one output port (`ocr.spellcheck``correctedResult` / `corrections` / `metadata`; or `document.classify`, `azureOcr.poll`). After a Try, look at that node’s preview. **Pass:** a row of small port chips appears above the preview, labelled with the catalog port labels; the first port is selected and its value is shown; clicking another chip switches the pane to **that port’s** value, styled by that port’s own kind. A single-output node shows **no** chip row (unchanged). (G-011)
474+
- [ ] **9.5b Bound-but-empty and unbound outputs.** *(unbound clause verified verbatim 2026-07-27; bound-but-empty not yet)* On a node whose output binding points at a ctx key the run never wrote: **Pass:** “No value was recorded for this output (expected `<Kind>`).” On a node with no output binding at all: **Pass:** “This step’s output isn’t bound to a workflow value yet, so there’s nothing to read.” Neither may be a blank card. (G-011)
475+
- [ ] **9.5c OCR previews show values, not a blob pointer.** *(payload-values + truncation clauses verified 2026-07-27; the blob-deleted clause not yet)* On an OCR step (`azureOcr.extract`, `mistral-ocr.process`, any `ocr.*` correction). **Pass:** the K/V table shows the **payload’s** keys (e.g. `fileName`, `status`, `extractedText`, `pages`, `documents`) — **not** `blobPath` / `storage` / `byteLength`. For a custom-model run, drilling into `documents → fields` via **View raw** shows the extracted field values. If the payload was bounded, a dimmed line reads “Truncated preview — pages: showing the first 5 of N items; …”, naming every omission. If the blob is gone (delete it from MinIO and re-open the preview), **Pass:** the pointer is shown **with** “The full OCR payload is no longer in storage — showing the reference only.” — never a bare blob key with no explanation. (G-022)
476476
- API: `GET /api/workflows/<WF>/preview-cache?nodeId=<NODE>` → response carries `blobExcerpts` keyed by `blobPath`, each with `status`, `truncated`, `omissions[]` and the `limits` applied.
477477
- [x] **9.6 Incremental re-run (cache).** Run → tweak one node param (e.g. `confidenceThreshold`) → Try again same input. **Pass:** unchanged upstream nodes flash **violet (cache hit/skipped)**; tweaked node + downstream re-execute; that preview updates. ⚠️ To force a miss there’s no UI — `DELETE FROM activity_output_cache WHERE node_id='<id>'`.
478478
- [x] **9.7 Cancel-on-new-Try.** Start a Try, then Try again mid-run. **Pass:** prior run cancelled server-side (shows **cancelled** in Run history); exactly one active run.
@@ -482,11 +482,11 @@ Requires Temporal server + **worker** + visibility store + `activity_output_cach
482482
- [x] **9.9a Replay shows the graph that RAN (G-004).** Run a workflow, then **change it** — add a node, delete one, save (creating a new version). Now **More ▸ Run history ▸ Replay** the earlier run. **Pass:** the canvas renders the **older version's** graph: the node you added is **not** on it, and the node you deleted **is** (wearing its result). The replay chip names that version (`v2`, not head). *(Before G-004 statuses were matched by id and painted onto whatever config was on screen — you read yesterday's results on today's diagram.)* Click **Clear**: the canvas returns to the live graph, including the added node.
483483
- [x] **9.9b Replay never risks unsaved work (G-004).** Make an **unsaved** edit (rename a node — do NOT save). Enter replay per 9.9a, try to edit something on the historical graph, **press Undo while still in replay**, then **Clear**. **Pass:** your unsaved rename is still there, the replayed version's content did **not** leak into the editing config, the leave-page guard still warns, Undo/Redo are **disabled** while replaying, and once you leave replay **Undo** steps back through your own edits only (replay adds no history entries).
484484
- [x] **9.9c Version that can't be loaded.** Replay a run whose version was removed (or stop the backend mid-replay). **Pass:** the replay chip turns orange and reads **"v{n} unavailable, showing current graph"** — it never silently pretends the current graph is the one that ran.
485-
- [ ] **9.10 Cache-evicted preview.** Replay a run whose cache row was deleted **for a node that SUCCEEDED** (`DELETE FROM "ActivityOutputCache" WHERE "nodeId"='<id>'`). **Pass:** red alert “Preview unavailable — cache evicted. Re-run **v{n}** (the version you are viewing) to repopulate.” + a **Re-run v{n}** button that fetches the original `initialCtx` and starts a fresh Try. This must remain distinct from 9.10a — eviction is the ONLY no-output state that offers a Re-run.
485+
- [x] **9.10 Cache-evicted preview.** Replay a run whose cache row was deleted **for a node that SUCCEEDED** (`DELETE FROM "ActivityOutputCache" WHERE "nodeId"='<id>'`). **Pass:** red alert “Preview unavailable — cache evicted. Re-run **v{n}** (the version you are viewing) to repopulate.” + a **Re-run v{n}** button that fetches the original `initialCtx` and starts a fresh Try. This must remain distinct from 9.10a — eviction is the ONLY no-output state that offers a Re-run.
486486
- [x] **9.10b Re-run targets the version being viewed (G-024).** With 9.9a's setup (a run on an older version, plus newer edits saved as head), trigger the Re-run from 9.10 while replaying. **Pass:** the POST body carries `workflowVersionId` for the **replayed** version, and the resulting run's history row pins that version — not head. Outside replay (no version pin) the button reads plain **Re-run** and targets head, which is correct there. *(Before G-024 the offered remedy silently ran a different graph and filed the result as if it were the same thing.)*
487487
- API: watch `POST /api/workflows/<WF>/runs` in devtools → `{ "initialCtx": {…}, "workflowVersionId": "<the replayed version>" }`.
488488
- [x] **9.10c Retention (G-024).** Intermediate values are kept **14 days** by default, not 24 hours — "the run happened yesterday" is the most common debugging situation there is and used to land exactly on the old boundary. **Pass:** `SELECT "createdAt", "expiresAt" FROM "ActivityOutputCache" ORDER BY "createdAt" DESC LIMIT 5;` shows `expiresAt ≈ createdAt + 14 days`, and replaying a run from **two days ago** still shows its previews rather than the cache-evicted alert. Set `ACTIVITY_OUTPUT_CACHE_TTL_MS=3600000` in the worker/backend env and restart: freshly written rows now expire in an hour, proving the window is tunable without a code change. ⚠️ The GC sweep is operator-started (`cache-gc-singleton`); expiry is enforced lazily on read regardless, so an unswept row past `expiresAt` still reads as evicted.
489-
- [ ] **9.10a Distinct no-output states.** Each situation below must show its **own** copy, and none may offer a Re-run button. (G-012)
489+
- [ ] **9.10a Distinct no-output states.** *(control-flow clause verified 2026-07-27 on 3 nodes; the other five states not yet)* Each situation below must show its **own** copy, and none may offer a Re-run button. (G-012)
490490
- **Mid-Try, node not reached yet:** “Waiting — the run hasn’t reached this step yet.” (Previously this was a blank card — the live run showed nothing at all.)
491491
- **Mid-Try, node executing:** “Running now — output appears when this step finishes.”
492492
- **Replay, branch not taken:** on a switch-heavy workflow, replay a run and look at a node on the untaken branch → “This step was never reached — the run took a different branch.”

0 commit comments

Comments
 (0)