Skip to content

docs(automation): record why failed has no column on sys_automation_run, and name it in summary_json - #16759

Merged
os-trump merged 4 commits into
mainfrom
claude/issue-15606-failed-stays-in-the-blob
Sep 8, 2026
Merged

docs(automation): record why failed has no column on sys_automation_run, and name it in summary_json#16759
os-trump merged 4 commits into
mainfrom
claude/issue-15606-failed-stays-in-the-blob

Conversation

@os-trump

@os-trump os-trump commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Fixes #15606

Option 2 of the card, per the ruling on comment 5567696655 (director seat, decision batch #76, 2026-09-07; maintainer verbatim 「同意」): no failed_count column now. FlowRunSummary.failed stays inside the summary_json blob on sys_automation_run, and the schema now says why.

⛔ This is not "do nothing". The ruling's test of done is its own sentence — "the asymmetry is explained where the next reader will look, so the card is not re-filed" — so the deliverable is judged by placement, not by word count.

Where the explanation went, and why that is where the next reader looks

packages/services/service-automation/src/sys-automation-run.object.ts, appended to the comment block that already sits directly above selected_count.

That block is not a neighbouring location — it is the cause of the card. Its first paragraph is the "COLUMNS, not just a blob … the difference between a dashboard and an alarm" argument, which triage measured at :312 with selected_count at :333 and acted_count at :340. A reader arrives at that paragraph, learns that four totals are columns because an operator can only alert on what is filterable, and then reads the four field declarations that follow it. Noticing that a fifth total exists and has no declaration among them is a consequence of having just read that paragraph, not an independent act. The answer is now in the same block, before the four declarations, so the question and its answer cannot be reached separately.

Concretely, the new prose states the ruling's own criterion: the four are columns because ONE filter expression needs them in ONE row (selected_count > 0 AND acted_count = 0, qualified by unmeasured_count), a WHERE clause cannot reach into a JSON blob for an operand, and failed is not one of that expression's operands — it would be its own predicate, nobody alerts on it today, and a caller that wants it has already fetched summary_json.

Two supporting placements, because "where the reader will look" is not one address:

  • summary_json's own description now names failed as the field to read lost-row counts from, states that the run-level totals live in the blob beside the per-node breakdown, and repeats the "absent is not zero" convention. This is the deliverable's second half: the prose above tells a reader the count is in the blob, and this is what makes the blob a usable answer instead of a redirection.
  • ObjectStoreSuspendedRunStore's terminal-row write (suspended-run-store.ts), where four record.summary?.X ?? null lines sit and a fifth would be added, now carries a five-line signpost to that verdict. ⚠️ Declared as a deliberate step past the ruling's literal "prose in one object file": a reader at the write site sees exactly the same asymmetry, and this is the address a future implementer of the re-open card lands on. It re-argues nothing — it points. Trivially dropped if the reviewer wants the diff held to the one file.

One placement deliberately NOT made

packages/spec/src/automation/execution.zod.ts, where FlowRunSummary.failed is declared, was considered and rejected — recorded here so the omission reads as a decision rather than an oversight. Two reasons, and the first is the load-bearing one: the asymmetry is not visible there. That file declares failed and unmeasured as peers, both optional, with no columns anywhere in sight; nothing in it can prompt "why does this one lack a column?", because it has no columns to be asymmetric with. The question only exists where the four declarations are. Second, it is packages/spec — a heavier surface carrying generated artifacts — and the ruling scoped the dispatch to "prose in one object file plus a pin".

The pin

New file packages/services/service-automation/src/sys-automation-run-failed-count-verdict.test.ts. Four cases:

  1. the four counters that a single filter expression needs in one row are still columns — this is also the positive control for case 2, since it exercises the same accessor path;
  2. no failed_count column, and no fail-named column under any other spelling. The name matcher carries its own positive control inside the test: the same matcher, over the same key list plus failed_count, fires;
  3. summary_json's description still contains `failed` — the pin the card asked for, so the explanation cannot rot into a claim the schema no longer supports;
  4. failed_count is not in highlightFields while acted_count still is.

Ablation — both directions, and one void reading reported

Ablation ran against object-file blob 2cb00e1a1b4dda0274f580ef80325729ff930c11, which is byte-identical to that file at this PR's head, so the reading below is about the tree being reviewed.

  • Control, unmutated treeTest Files 1 passed (1) / Tests 4 passed (4).
  • Leg 1 — add a failed_count fieldTests 1 failed | 3 passed (4), the failure being exactly the case named declares no failed_count column — the verdict itself, with AssertionError: expected { …(12) } to be undefined.
  • Leg 2 — blank summary_json's descriptionTests 1 failed | 3 passed (4), the failure being exactly the case named summary_json description NAMES failed as the place to read lost-row counts, with AssertionError: expected 'ABLATION: description with no counter…' to contain 'failed'.

Each leg reddens exactly one assertion and leaves the other three green, so the pin is targeted rather than a blunt instrument. Mutations were proven on disk by grep counts before and after plus a git hash-object differing from the HEAD blob; each restore is verified by on-disk hash equal to the HEAD blob and git diff HEAD empty, through an EXIT INT TERM trap using absolute paths.

⚠️ Reported because it changes how the above should be read: a first ablation attempt produced exit 1 on both legs and looked like a clean pair of red readings. It was not — the run had passed vitest a repo-relative test path while vitest runs with its cwd at the package, so the real output was No test files found, exiting with code 1. Nothing was measured, and both readings were discarded rather than reported. The numbers above are from the corrected re-run, which is why the control leg and the per-assertion names are quoted: an exit code alone could not have told the two apart.

Both legs ran with no rebuild, which is sound here and was checked rather than assumed: the pin reaches its subject through the relative import ./sys-automation-run.object.js, and this package's only vitest alias targets @objectstack/platform-objects, so resolution lands on src, never dist. No dist preflight applies.

The re-open condition, as recorded

Written into the same comment block, in the ruling's terms and marked as deliberate ("stated so it is not a matter of taste later"): the FIRST real need to alert on "which runs lost rows this week" is the card that adds failed_count, mirroring unmeasured_countnull on rows written before the column existed, ⛔ never 0, because "not tracked" and "nothing failed" are different answers and only one of them should stay quiet. One column on an ADR-0103 engine-owned object: a human-floor change, never a rider. It is also restated in the changeset and in the pin's docblock, which names it as one of exactly two legitimate ways to turn the file red.

Clause-② — re-derived, no

The dispatch's read was provisional and told me to re-derive it from the real export surface, and specifically to measure whether the description strings ship. They do:

  • @objectstack/service-automation is published (publishConfig.access: public, files: ["dist"]) and SysAutomationRun is exported from src/index.ts.
  • Built, then grepped dist/: the new description text is present in both dist/index.js and dist/index.cjs (1 hit each). Absent from dist/index.d.ts (0), as expected for a runtime value. tsup does not minify, so the new source comments ship as well — verified with a single-line comment phrase confirmed single-line in the source first, plus a grep -Pzo multiline cross-check for a phrase that wraps.

Shipping in the artifact is not the same as being a contract, and that is where the no comes from:

  • No new exported symbol, no new field, no new key on any published payload. The field-name set is 28 names on origin/main and the same 28 here, diff clean — same types, same required flags; every accepted set is byte-for-byte what it was.
  • The only published byte change is prose inside an already-declared description value, plus source comments. Nothing parses it: git grep -l for the description text resolves to exactly one tracked file, the object file itself. Firing positive control for that same matcher: git grep -l 'unmeasured_count' returns 6 files, so the single-file result is a measurement and not a broken search. It is in no generated baseline or snapshot either.
  • The pin is a test file: not an entry point, not in dist, not published.
  • Mechanical support: node scripts/pm/check-widening-tells.mjs --declaration no --diff PR-DIFF-FILE ⇒ exit 0, "4 changed file(s) read, no widening tell on any declared surface".

Clause-②: no. No needs:contract-review label, no --pair. Not a governed surface either: today's register prints docs/adr/** · .claude/** · skills/** · AGENTS.md · CLAUDE.md, and none of the four paths hits one.

Changeset — patch, ⛔ not skip-changeset

AGENTS.md:1029 scopes skip-changeset to "a diff that publishes nothing from any released package". Measured above: this diff changes bytes inside the published bundle of a released package. So the label is refused and .changeset/automation-run-failed-stays-in-the-blob.md declares patch, and it now records how that was measured rather than asserting it. description is additionally what the authorable help / helpText keys alias onto in packages/spec/src/data/object.zod.ts, so it is documentation a consumer surface can render, not an internal note. Not breaking — nothing authorable is removed or renamed — so no ADR-0087 marker applies, and check-adr-0087-registration.mjs --base origin/main is green.

Verification

⚠️ origin/main moved twice during this work and dispatch-gates reported STALE TREE each time; it was fetched and merged on both occasions and the gate list re-derived before anything was run. Everything below is at head eab69961d, whose merge base with origin/main is current.

  • pnpm --filter '@objectstack/service-automation^...' build ⇒ exit 0 (dependency closure first, so a stale dist cannot fake either direction), then pnpm --filter @objectstack/service-automation build ⇒ exit 0.
  • pnpm --filter @objectstack/service-automation test126 files, 1490 tests, all passed.
  • pnpm --filter @objectstack/service-automation typecheck ⇒ exit 0 (tsc --noEmit plus check:test-typecheck, the latter reporting the test layer compiles with 0 files / 0 errors in the debt ledger). The new pin is genuinely inside that program, not merely adjacent to it: tsc -p tsconfig.test.json --listFiles names it (1 hit).
  • node scripts/pm/dispatch-gates.mjs --tier ⇒ no path-derived model mandate.
  • --commands ⇒ 56 families. All 56 ran, all exit 0, and --ran reconciled: "56 derived famil(ies) accounted for — 56 run, 0 NOT-MEASURED".
  • ⚠️ That full sweep ran at the tree before the second origin/main merge. Because two gate implementations arrived in that merge, the sweep was restarted at this head; 52 of 56 re-ran green here, 0 non-zero throughout. The 4 not yet re-run at this exact head are check:type-check-debt, check:type-source-resolution, check:watch-hint-literal and check:where-matcher — all green in the full sweep, all unchanged implementations, and none of them reads a file this merge touched. Declared as a narrowing, not claimed as measured; CI runs the full farm.
  • The narrowing is safe on the point that motivated the restart: of the two gate implementations the merge changed, scripts/engine-double-contract.pinned.json backs check:engine-double-contract, which did re-run green at this head, and scripts/check-partof-closing-keyword.mjs is not in the runnable 56 at all (it sits in the self-test/roster bucket). The re-run also re-verified the ESLint ratchet's baseline against the new merge base 8b37a09 (the first sweep had verified against 73053ed) — green, "none new".
  • Every gate's exit code was captured before any pipe (cmd > log 2>&1; EXIT=$?), never through | tail or | head.
  • The three heaviest did real work rather than skipping: check:type-check-debt re-measured 5 ledger entries (55 raw tsc errors, none above its recorded number), check:dual-build-cjs-loads loaded 104 published require entry points across 67 packages, check:dts-closure swept 164/164 declaration files across 72 packages.
  • Control characters: grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]' over all four changed files finds none.

Repo-wide eslint and the always-runs CI tail are left to CI, which runs the full farm.

验收备注


🤖 Generated with Claude Code

https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37

…n_run`, and name it in `summary_json`

For #15606, decision batch #76 (2026-09-07): option 2 — `failed` stays in the
`summary_json` blob, and the schema says why.

`FlowRunSummary` carries five run-level totals; four of them have a column on
`sys_automation_run` and `failed` does not. The four are columns because ONE
filter expression needs them in ONE row — `selected_count > 0 AND
acted_count = 0`, qualified by `unmeasured_count` — and a WHERE clause cannot
reach into a JSON blob for an operand. `failed` is not one of its operands: it
would be its own predicate, nobody alerts on it today, and a caller that wants
it has already fetched `summary_json`.

The reasoning goes in the comment above `selected_count` — the same paragraph
that explains why the four ARE columns, and therefore the paragraph a reader is
already in when they notice the fifth is not — together with the re-open
condition: the first real need to ALERT on "which runs lost rows this week" is
the card that adds `failed_count`, mirroring `unmeasured_count` (null on rows
written before the column existed, never `0`), a one-column change on an
ADR-0103 engine-owned object and a human-floor one.

`summary_json`'s description now names `failed` as the field to read lost-row
counts from, and `sys-automation-run-failed-count-verdict.test.ts` pins both
halves — no `failed_count` (or any other `fail`-named) column, and a
description that still names `failed` — so the explanation cannot rot into a
claim the schema no longer supports. The terminal-row write in
`ObjectStoreSuspendedRunStore`, where a fifth `record.summary?.failed ?? null`
line would go, points at that verdict so it is not re-derived from the write
site either.

No schema shape moves: no field added, removed or renamed, no type or
`required` flag changed, and every accepted set is byte-for-byte what it was.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
@github-actions github-actions Bot added the size/m label Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 6 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 8b37a0973d20b417569f934ad5545e2a39dca257packageMentionDocs.

Which tree this was computed on

This run read content/docs from ff6e3700212c11787df288558e498559e85709d1 — the merge of head eab69961da0f02598e111074985d74ae6b6f460a into base 8b37a0973d20b417569f934ad5545e2a39dca257, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin ff6e3700212c11787df288558e498559e85709d1 && git checkout ff6e3700212c11787df288558e498559e85709d1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8b37a0973d20b417569f934ad5545e2a39dca257 eab69961da0f02598e111074985d74ae6b6f460a && git checkout -B drift-repro 8b37a0973d20b417569f934ad5545e2a39dca257 && git merge --no-ff eab69961da0f02598e111074985d74ae6b6f460a

node scripts/docs-audit/affected-docs.mjs --json 8b37a0973d20b417569f934ad5545e2a39dca257

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 8, 2026
…hips

The changeset asserted the `summary_json` description reaches the published
bundle; it now states HOW that was measured rather than asserting it, because
that measurement is the whole reason this diff is not `skip-changeset`:
`SysAutomationRun` is re-exported from `src/index.ts`, `package.json` publishes
`files: ["dist"]`, and after a real build the new text is present in both
`dist/index.js` and `dist/index.cjs`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] service-automation: FlowRunSummary.failed has no queryable column on sys_automation_run, so "which runs lost rows?" is a summary_json scan

2 participants