docs: the census total is a floor — 25 membership predicates it structurally cannot see, one a live defect - #2763
Merged
Conversation
…on is invisible to it Docs only. Extends the entry #2748 landed, because the fleet reads the census total as the completion bar and that total structurally excludes a whole predicate class. MEASURED on origin/main: - 47 array/Set literals of two or more lifecycle ids, in 35 files. - 25 of those are MEMBERSHIP PREDICATES against a task's column — `SET.has(task.column)` / `ARRAY.includes(task.column)` — in 19 files. Two are documented fallbacks behind a resolved primary, so ~23 are unconverted guards. - The census scans `===`/`!==` against a column. None of these is a comparison, so none is counted. They behave exactly like the guards it does count, and ONE IS A PROVEN LIVE DEFECT: `isImplementationTask` is `IMPLEMENTATION_TASK_COLUMNS.has(task.column)`, and `evaluateImplementationTaskBind` short-circuits to `allowed: true` when it is false — so on a renamed board every agent is bind-compatible with every task and the role check that stops a liaison being handed implementation work does not apply. It surfaced only because a reviewer questioned a coverage claim in one of my dispatch tests (#2739); asserting the claim properly turned it into a failing test. So the census total is a FLOOR, not a total. Not an argument against it — it is AST-based and honest about what it measures — but an argument against reading "backlog: N" as "N guards remain". Same shape as the archived gate (#2724), where the rule is also encoded in Drizzle predicates and raw `sql` templates. Extending the census to count membership predicates is deliberately left as a coordinator call: it would move every worker's number mid-fleet, and deciding which sets are lifecycle guards versus board-config definitions or type unions is exactly the judgement the DELIBERATE-LITERAL marker exists for. `pnpm lint` clean; census --strict exits 0 (no code changes). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThe PR expands the lifecycle-column census documentation and refreshes its baseline.
Confidence Score: 5/5The documentation-only PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| docs/solutions/test-failures/optional-flags-seam-hides-unconverted-column-guards.md | Adds and self-corrects the census blind-spot analysis, including explicit locations for the previously questioned TaskCard constant. |
| scripts/lib/lifecycle-column-census-baseline.json | Removes the packages/core/src/default-workflow-hooks.ts count from the generated census baseline. |
Reviews (4): Last reviewed commit: "docs: correct the membership-predicate c..." | Re-trigger Greptile
The finding is WRONG on the row it names, and checking it surfaced two rows that are genuinely wrong. `TIME_INDICATOR_COLUMNS` IS present in TaskCard.tsx — declared at 333, used at 1712 and 3070. Two membership uses, exactly as the table says. Nothing to fix there. But the same check applied to every other row found: cli/src/commands/task.ts retryReviewColumns 3 -> 4 engine/src/eval-followups OPEN_COLUMNS 2 -> 1 The counting rule was never written down, which is how both drifted: the numbers are MEMBERSHIP USES, not occurrences of the identifier, so the declaration line does not count. `IMPLEMENTATION_TASK_COLUMNS` at (1) is the row that proves the rule — two occurrences, one use. Recorded in the file as a comment with the one-line command that reproduces it, so the next reader re-measures instead of re-deriving. Worth stating plainly: a table of hand-counted numbers with no stated method and no check is a census with the instrument removed, which is the exact failure this document exists to describe. Verified: lint clean (0 errors), gate green (487 + 158 + 10 + 71). Docs only.
Collaborator
Author
|
The finding is wrong on the row it names — and checking it surfaced two rows that are genuinely wrong.
So there is nothing to fix there. But I applied the same check to every row rather than only the one questioned, and two were off by one:
Why both driftedThe counting rule was never written down. The numbers are membership uses, not occurrences of the identifier — the declaration line does not count. Without that stated, the next person re-derives it and gets a different answer, which is how a 2 became a 1. The file now carries the rule and the one-line command that reproduces every number: Worth saying plainly, since this document is about census accuracy: a table of hand-counted numbers with no stated method and no check is a census with the instrument removed. That the review's one objection was the single correct row, while two neighbours were wrong, is the argument for re-measuring the whole table rather than the cell someone points at. Verified: lint clean (0 errors), gate green (487 + 158 + 10 + 71). Docs only. |
… wrong in KIND, not number The #2763 review corrected the per-row counts. Re-measuring to answer it showed two rows should not have been in the table at all. `retryReviewColumns` and `sourceTerminal` are ALREADY RESOLVED — `columnsWithFlag(ir, ...)` with a legacy array as the no-IR fallback. My scan classified a constant by whether its initializer text contained only lifecycle ids, so the FALLBACK array matched and they read as hardcoded. Six sites across the tree were miscounted that way. Corrected measurement: 100 membership tests against a task column overall, most already resolving from the task's IR; 19 against hardcoded sets in 16 files; ~16 genuine unconverted guards once seeds and documented fallbacks behind a resolved primary are removed. Rows REMOVED rather than renumbered — a wrong kind is not fixed by a better count. This is the section's own thesis one level up: my instrument counted syntax and called it meaning, which is exactly what it criticises the census for. Recorded rather than quietly corrected, because the 25 was already published and used to argue about the completion bar. The load-bearing claims are unchanged: the class is invisible to a comparison-based scan, and `IMPLEMENTATION_TASK_COLUMNS` remains a proven live defect. `pnpm lint` clean; census --strict exits 0 (no code changes). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
The review's count corrections were right, and answering them showed two rows should not have been in the table at all.
Corrected headline: 25 → 19.
I removed those two rows rather than renumbering them: a wrong kind is not fixed by a better count, and leaving them with corrected numbers would have preserved the wrong claim in a more convincing form. This is the section's own thesis one level up — my instrument counted syntax and called it meaning, which is exactly what it criticises the census for. The corrected rule excludes any initializer deriving from an IR or a resolver at runtime. Recorded in the file rather than quietly fixed, because the 25 was already published and used to argue about where the completion bar sits. The load-bearing claims are unchanged: the class is invisible to a comparison-based scan, and
|
gsxdsm
added a commit
that referenced
this pull request
Jul 30, 2026
…worker's mega-batch) (#2773) The engine mega-batch. Folds my four engine PRs and will absorb the remaining `packages/engine` guards as commits on this branch. **Superseded and closed:** #2722, #2741, #2766, #2770. ## Census — files converted so far | file | before | after | |---|---:|---:| | `notification/notification-service.ts` | 9 | **5** | | `runtimes/in-process-runtime.ts` | 6 | **1** | | `eval-followups.ts` | 2 | **0** | | `pr-comment-handler.ts` | 1 | **0** | | `task-revert.ts` | 2 | **0** | The last two are **census-invisible** (`Set.has(task.column)` membership) — the class measured in #2763, which a comparison-based scan cannot count. So the backlog number moves less than the work does, deliberately. ## What each one actually fixed — all silent, none cosmetic - **Notifications stopped entirely.** `handleTaskMovedAsync` compared `data.to` to `in-review`/`done`, so on a renamed board the two notifications operators rely on most were never sent. - **A finished card's plan review could re-enter.** The continuation drain's terminal test matched nothing, so a completed card's planning continuation was handed to the executor. - **The revert route admitted and the service refused.** The route resolved terminal lanes; the service compared to a hardcoded pair. The operator got a dead end from an affordance the UI and route both offered. - **Follow-up dedup blocked new cards forever.** A finished follow-up in a renamed complete lane read as *open*, so the dedup matched it permanently — defeating the intent the code documents in the line above it. - **The mission requeue wrote a column that may not exist**, and its guard never matched. ## Flagged, not fixed — deliberately - **`concurrency.ts` idle semaphore leak recovery** — the last live caller of the running-agent predicate that does not enrich. On a renamed board it under-counts and can reclaim a legitimately-held slot. The enriching variant is async and this is a synchronous repair path whose failure mode is reclaiming live work. - **The archival `task:moved` listener** — runs on every move with no cheap gate ahead of it; converting costs an IR resolution per move to decide most moves are not archival. ## Notes carried from the folded PRs Two conflicts resolved in main's favour because **main's version was better**: `in-process-runtime`'s seam uses `terminalColumns: ReadonlySet` (membership) where mine used `LifecycleColumns` (first-per-role), and the test is rewritten against main's API. That arity trap has now caught me four times, so membership is the default shape in everything new here. Review fixes from the folded PRs are included: the notifier's review set, the second human-review site, the second dedup copy, the workspace revert surface, and the file-content assertions. ## Verification `pnpm test:gate` **GREEN** (158 + 10 + 487 + 71) · **224 passed** across the touched engine suites · engine and dashboard `tsc` clean · `pnpm lint` clean · census `--strict` exits 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
gsxdsm
added a commit
that referenced
this pull request
Jul 30, 2026
…n a renamed board The #2763 class — `Set.has(task.column)` membership, which no comparison-based scan counts, so the backlog number does not move at all for this commit. ephemeral-worker-manager `isAgentIdle` — BOTH halves were hardcoded, and the failure is inverted on each side: an agent working a LIVE card in the wip lane was reported idle and became eligible for teardown, while an agent bound to a FINISHED card was reported busy. worktree-pool — a branch is active unless the merger owns the card or it is archived. On a renamed board a card in review or complete was not recognised as merger-managed, so its branch was treated as reclaimable: deleting a branch out from under an in-flight merge. agent-assignment — assignment load counts how many active tasks each agent holds and is what balances new work. On a renamed board nothing counted, so every agent looked unloaded and the balancer piled work onto whichever sorted first. eval-followups OPEN_COLUMNS left as-is: it feeds a duplicate-detection scan whose list is already query-bounded, so the same reachability argument applies as in self-healing. `SelectPermanentAgentForTaskOptions.taskStore` widened from Pick<TaskStore, "listTasks"> to include the workflow readers. Its only production caller (scheduler.ts) passes a full TaskStore, so this narrows nothing — the Pick was documenting the subset used at the time. LINT CAUGHT THE TAIL: removing three now-dead constants surfaced an unused `ColumnId` type import. Worth noting because the census cannot see any of this — without lint, the dead constants would have stayed as convincing-looking evidence that the file still keys on ids. Gate 158+10+487+71 GREEN. 132 passed across the three suites. Engine tsc and lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gsxdsm
added a commit
that referenced
this pull request
Jul 30, 2026
…e` refused every card on a renamed board (#2775) `batch-cli-plugins` — the u7 worker's mega-batch: `packages/cli` + `plugins` + anything left. ## The batch is 7 guards, and 3 of them are not guards at all The census's per-file list gives this batch seven sites. Reading them, **three are a foreign vocabulary the census matches on the string alone**: | file | site | verdict | |---|---|---| | `plugins/fusion-plugin-reports/store/report-store.ts` | `next === "archived"` ×2 | **not a column** — `next` is a `ReportStatus` | | `plugins/fusion-plugin-reports/store/report-types.ts` | `to === "failed" \|\| to === "archived"` | **not a column** — same enum, its own terminal states | The reports plugin has its own status lineage (`draft → generating → review_* → approved → published`, plus `failed`/`archived`) that shares two spellings with the lifecycle vocabulary. A report is not on a board and has no workflow, so resolving an IR there would answer a question nobody asked. All three are marked `DELIBERATE-LITERAL` with the reason at the site. **This cuts the other way from #2763.** That PR establishes the census total as a *floor* (25 membership predicates it structurally cannot see). This is the opposite error in the same number: a foreign enum inflating it. The total is neither a ceiling nor a floor — it is an estimate with error in both directions, and the per-file list is worth reading before trusting a file's count. ## Converted (census before → after, per file) | file | before | after | |---|---|---| | `packages/cli/src/commands/pr.ts` | 1 | **0** | | `plugins/…/even-realities-glasses/notifications/diff.ts` | 1 | **0** | | `plugins/…/reports/store/report-store.ts` | 2 | **0** (deliberate) | | `plugins/…/reports/store/report-types.ts` | 1 | **0** (deliberate) | ### `fn pr create` refused every card on a renamed board The live defect in this batch. The gate was `task.column !== "in-review"`, and its error told the operator to move the task to a column their board does not have: ``` Error: Task must be in 'in-review' column to create a PR (current: signoff) ``` There is no way to satisfy that short of renaming the workflow back. Now resolved through core's `resolveReviewColumns`, and the message names the lanes that actually exist. **The SET, not `lifecycle.review`.** A board may declare more than one review lane, and a card parked in a `humanReview`-only lane is still a card you can open a PR from. A single-id answer keeps refusing those — the same narrowing #2728's review caught in the CLI retry gate, which is why the test pins both lanes. ## Skipped, with the reason **`plugins/fusion-plugin-even-cards` (2 guards) — blocked on packaging, not on analysis.** The defect is real: `boardToDeck` filters with `column !== "archived" && column !== "done"`, so on a renamed board every finished card stays in the deck, fills `maxCards`, and pushes the active cards off the display. The wearer sees a board that never finishes anything. I implemented the fix and **reverted it**: this plugin is not in `pnpm-workspace.yaml` and depends only on `@fusion/plugin-sdk` — it has no `@fusion/core` dependency, so the route cannot reach `resolveTaskLifecycleColumns`. Adding one is a packaging change, which this program's rules put out of scope. Shipping only the injected parameter without a caller was the alternative, and that is precisely the decorative conversion #2759 documents: the census would drop by 2 and the deck would keep the bug. Flagged for whoever owns the plugin's dependency surface. The glasses plugin next door *does* depend on `@fusion/core`, so this is a one-plugin problem, not a plugin-wide one. ## Honest note on the glasses conversion `diff.ts`'s completion branch is **currently unreachable** — the only production caller (`notifier.ts`) passes `alsoNotifyOnDone: false`. So that conversion changes nothing at runtime today. It is converted rather than marked deliberate because the literal is not deliberate: it is wrong, and would ship the bug the day someone turns the flag on. Stated here rather than left for a reviewer to discover. ## Verification - new CLI suite **4 passed**; `pr-command` + `pr-automerge-cleanup` + `bin-pr-router` **35 passed** - glasses plugin **181 passed (19 files)** · reports plugin **110 passed (23 files)** - `pnpm test:gate` — **158 / 10 / 487 / 71** · `pnpm lint` clean · `--strict` exits 0 **Revert proof, measured.** Restoring `if (task.column !== "in-review")` fails 3 of the 4 new cases (`process.exit:1` on both renamed lanes, and the refusal message reverts to naming `in-review`). The unresolvable-workflow case keeps passing — it is the legacy path — so the negative cases alone do not pin the fix and all four are required. ## Handoff to `batch-engine` `packages/engine/src/project-engine.ts` **5 → 0** is finished, green, and pushed as `handoff/project-engine-lanes-for-batch-engine` (`34dbb35209`) for the capacity worker to cherry-pick — it is engine-owned, not mine to land. It fixes two live defects: a card that **had merged** reported as a failed merge to `fn task merge` and the dashboard button (`merged: finalTask?.column === "done"`), and the three post-finalize `column === "done" && mergeConfirmed` fast-path checks, which on a renamed board sent an already-landed card down the bounce path — re-queued, retry-counted, and in the capped branch parked `failed` with its merge sitting on main. Plus `hasAutoHealableVerificationBufferFailure`, which returned false for every card on a renamed board, so a buffer-overflow verification failure was never auto-healed. 8 new tests, revert-proven (restoring the literal fails 4 of 8), gate green. --- ## Completion pass (u7) — the batch is now closed Two workers converged on this branch. I rebased onto the first-landed commit rather than force-pushing over it, took its wording wherever the conclusion was identical, and added what was missing. ### What this pass added 1. **`even-cards` (2 sites)** — the only in-scope file the first pass left open. Marked DELIBERATE-LITERAL: the package depends on `@fusion/plugin-sdk` only, and the SDK does not re-export the lifecycle role helpers, so there is no IR, no store, and no trait flags to resolve *from*. Fixing it properly means the SDK exposing role flags on the task shape it hands plugins — a structural change, out of scope, and recorded at the site as the correct home. Live consequence is cosmetic: a finished card on a renamed board shows as active in the glasses deck. 2. **A red test in the `fn pr create` conversion.** The incoming version rendered `Task must be in 'in-review' to create a PR`, dropping the word `column`. `task.test.ts:3422` pins `must be in 'in-review' column`, so that hunk failed `runTaskPrCreate > exits with error when task not in in-review column`. Restoring the word makes the single-lane message **byte-identical** to the pre-conversion one, which is what a vocabulary conversion should be — the guard's own test now passes unmodified. Marked at the site so it is not "simplified" back. 3. **Duplicate imports** — the two independent conversions each added `resolveWorkflowIrForTask`/`resolveReviewColumns`, which does not compile. Deduped in its own commit. ### Census Measured with `--json` on `origin/main` and on this branch. | file | before | after | action | |---|---|---|---| | `packages/cli/src/commands/pr.ts` | 1 | 0 | converted | | `plugins/fusion-plugin-reports/src/store/report-types.ts` | 1 | 0 | marked | | `plugins/fusion-plugin-reports/src/store/report-store.ts` | 2 | 0 | marked | | `plugins/fusion-plugin-even-cards/src/cards/board-cards.ts` | 2 | 0 | marked | | `plugins/fusion-plugin-even-realities-glasses/.../diff.ts` | 1 | 0 | marked | Backlog **415 → 408** (−7, exactly the in-scope count). Deliberate **40 → 46** (+6 marked); 6 + 1 converted = 7. `--strict` exits 0. **Nothing remains in `cli` + `plugins` + everything-else — there is no follow-up batch behind this one.** ### One note on the `even-realities-glasses` site Worth recording beyond "cannot resolve": its only production caller (`notifier.ts:80`) passes `alsoNotifyOnDone: false`, so that arm is **unreachable today**. Converting it could not have changed observed behaviour either way. ### Verification (measured, on the merged branch) - `pnpm --filter @runfusion/fusion exec tsc --noEmit` → exit 0 - `pnpm lint` → 0 errors - CLI `task.test.ts` → 144 passed, including the `runTaskPrCreate` guard test - `@fusion-plugin-examples/reports` → 110 passed; `even-realities-glasses` → 181 passed **Pre-existing failures, not from this change:** the 5 `runTaskImportFromGitHub` / `runTaskImportGitHubInteractive` tests fail identically on `origin/main` — verified by stashing this diff and re-running (5 failed / 144 passed both ways). --- ## Census audit (unowned follow-on) After closing the batch scope I audited whether the **392** column-backlog number is inflated by foreign vocabularies — the class this batch found in the reports plugin, where `"archived"` is a `ReportStatus` rather than a board lane. If that class were widespread, every remaining batch would be chasing sites that must not be converted. **It is not. The number is real.** A receiver-level pass over all 392 column-category sites found exactly **3** false positives, all in `plugins/fusion-plugin-reports` (`next`, a `ReportStatus`), all now marked in this PR. What was checked and cleared: - **Property-reached foreign enums** (`step.status`, `feature.status`, `mission.status`) — already correctly bucketed into the separate `status` category (185), not the column backlog. Verified against `merge-queue-ops.ts`: 11 lifecycle-spelled literals in the file, census counts **1**, and that 1 is the genuine `.column` guard. - **Bare step-status variables** (`status`, `currentStatus`, `liveStatus` compared to `"done"`/`"skipped"`) — likewise excluded. - **Every other receiver in the backlog** — `to`, `from`, `column`, `fromColumn`, `toColumn`, `latestColumn`, `state`, `preArchiveColumn`. All resolve to genuine task columns. `executor.ts`'s 15 sites were spot-checked line by line: all 15 are real. The gap the classifier genuinely cannot close is a foreign enum held in a **bare variable** — the receiver name carries no type information, so `next === "archived"` is indistinguishable from a lifecycle guard by AST alone. That is why the reports sites need a marker rather than a classifier fix, and it is now documented in `lifecycle-column-census-ast.mjs`'s header alongside the measured scope, so the remaining batches do not re-run this hunt. Census tests: **43 passed**. The change is comment-only. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Docs only, extending the entry #2748 landed. Opening it because the fleet reads the census total as its completion bar, and that total excludes a whole predicate class — a measurement that should not live in a chat reply.
Measured on
origin/mainSET.has(task.column)/ARRAY.includes(task.column)— in 19 files. Two are documented fallbacks behind a resolved primary, so ~23 are unconverted guards.===/!==against a column. None of these is a comparison, so none is counted.cli/src/commands/task.ts(3)retryReviewColumnsdashboard/app/components/TaskCard.tsx(2)TIME_INDICATOR_COLUMNSengine/src/eval-followups.ts(2)OPEN_COLUMNSengine/src/merger.ts(2)sourceTerminalengine/src/task-revert.ts(2)REVERTABLE_COLUMNScore/src/agent-role-policy.ts(1)IMPLEMENTATION_TASK_COLUMNSOne is a proven live defect
isImplementationTaskisIMPLEMENTATION_TASK_COLUMNS.has(task.column), andevaluateImplementationTaskBindshort-circuits toallowed: truewhen it returns false. On a renamed board every agent is bind-compatible with every task — the role check that stops a liaison being handed implementation work (the NEXT-871 loop FN-7851 fixed) does not apply.It surfaced only because a reviewer questioned a coverage claim in one of my dispatch tests (#2739). Passing an agent wasn't proof the evaluator ran, so I asserted a
custom-role agent must be refused — and that test failed against production. Flagged at the site in #2739, not fixed:isImplementationTaskis a sync pure predicate with no store, and making the routing policy async is a behaviour change to agent admission.What this does and does not argue
The census is the right instrument — AST-based, honest about what it measures, and it has caught real drift in both directions (it failed on me in #2724 when merged conversions moved an inventory down). This is not an argument against it.
It is an argument against reading "backlog: N" as "N guards remain". The same shape already appeared in the archived gate (#2724), where the rule is additionally encoded in Drizzle predicates and raw
sqltemplates that no comparison scan can see. Two independent classes now, found the same way — by looking at what the instrument's definition excludes.Extending the census to count membership predicates is deliberately left to you, not done here. It would move every worker's number mid-fleet, and deciding which sets are lifecycle guards versus board-config definitions or type unions is exactly the judgement
DELIBERATE-LITERALexists for — 47 collections would each need that call.Verification
pnpm lintclean · census--strictexits 0 · no code changes.🤖 Generated with Claude Code