From 7f3eee9db7e00b3aebd0ce8dede534596b887aa6 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 08:31:47 -0700 Subject: [PATCH] =?UTF-8?q?docs(engine):=20mark=20the=20usage-limit=20term?= =?UTF-8?q?inal=20filters=20DELIBERATE-LITERAL=20=E2=80=94=20a=20documente?= =?UTF-8?q?d=20false=20positive=20that=20has=20now=20baited=20two=20worker?= =?UTF-8?q?s=20(#2767)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## No behaviour change. This is the work order retracting a documented false positive. I went to convert the `done`/`archived` filters in `usage-limit-detector.ts`, reasoning that on a renamed board a provider rate limit would pause already-finished work. I wrote the conversion — and only then read the note a previous worker had left directly above it: > *"The FIRST thing I suspected there — the `done`/`archived` terminal filter — turned out to be a **FALSE POSITIVE**: its revert stayed green, because the lane check already excludes finished cards."* **They are right and I was wrong.** A terminal card is already excluded downstream: `taskUsesProvider` resolves the task's active lane, a finished card matches no active lane, so it resolves no providers and cannot be affected. The suite pins exactly this — `pauses a PEER executing in the renamed WIP column` asserts `FN-SHIPPED` is not paused. My conversion is reverted. It changed nothing at runtime and would have lowered the census count while behaviour stayed identical — the precise shape this program keeps warning about, produced by me this time. ## Why a marker and not just the existing prose The note was already there and I walked into it anyway, because **the census kept listing this file as 4 unconverted guards**. The work order advertised the work; the reasoning against it lived in a comment you only reach after you have started. Prose informs a reader who is already looking; a marker informs the *instrument*, so the file drops out of the work order. Two distinct reasons are recorded rather than one blanket marker, because they are not the same argument: - **the prefilter** is a deliberate cheap **superset** (#2672 review). Converting it reintroduces the whole-board resolution that review removed. Literals are safe here in the direction that matters — a renamed board declares no `done`/`archived` id, so nothing is wrongly *excluded*. - **the final filter** is redundant with the lane check, and that redundancy is already proven by an existing test. ## Census | | before | after | |---|---|---| | `usage-limit-detector.ts` | 4 | **0** | | repo backlog | 437 | **433** | | DELIBERATE-LITERAL (reviewed) | 38 | **42** | Every one of the 4 is a marker, not a conversion. The backlog moved because reviewed literals left it honestly, not because behaviour changed. ## Verification `usage-limit-detector.test.ts` **58 passed**, unchanged before and after · `pnpm test:gate` **10 / 158 / 487 / 71** · `pnpm lint` clean · engine `tsc --noEmit` **0 errors** · `--strict` exits 0. Co-authored-by: Claude Opus 5 (1M context) --- packages/engine/src/usage-limit-detector.ts | 22 +++++++++++++++++++ .../lib/lifecycle-column-census-baseline.json | 3 ++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/packages/engine/src/usage-limit-detector.ts b/packages/engine/src/usage-limit-detector.ts index 6c4dfa2136..111d5d8b06 100644 --- a/packages/engine/src/usage-limit-detector.ts +++ b/packages/engine/src/usage-limit-detector.ts @@ -256,6 +256,14 @@ export class UsageLimitPauser { serialising to fix it would trade a bounded duplicate read for latency on the pause — which is the operation an operator is waiting on. Named rather than silently accepted. */ + /* + DELIBERATE-LITERAL — a cheap SUPERSET prefilter, kept literal on purpose (#2672 review). + + Its only job is to avoid resolving the whole board. Keeping literals is safe in the direction + that matters: a renamed board declares no `done`/`archived` id, so nothing is wrongly EXCLUDED + and every plausible card is still resolved. Converting it would reintroduce exactly the + whole-board resolution that review removed. + */ const laneCandidates = tasks.filter((task) => task.paused !== true && task.column !== "done" && task.column !== "archived"); await Promise.all(laneCandidates.map(async (task) => { @@ -283,6 +291,20 @@ export class UsageLimitPauser { preImplementationByTask.set(task.id, lanes); })); } + /* + DELIBERATE-LITERAL — REVIEWED AND PROVEN REDUNDANT, not overlooked. + + This is a documented FALSE POSITIVE for the lifecycle-column census, and it has now drawn in two + separate workers, which is why the marker is going on rather than only the prose above. A card in + a renamed terminal lane is ALREADY excluded downstream: `taskUsesProvider` resolves the task's + active lane, and a finished card matches no active lane, so it resolves no providers and cannot + be affected. The test suite states the same thing and pins it + (`pauses a PEER executing in the renamed WIP column` asserts `FN-SHIPPED` is not paused). + + So converting this changes NOTHING at runtime — it would be pure churn that lowers the census + count while the behaviour is identical, which is the shape this program keeps warning about. The + marker removes it from the work order so the next reader does not re-derive all of the above. + */ const affectedTasks = tasks.filter((task) => task.column !== "done" && task.column !== "archived" diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index 957dcb7411..4edd863ed8 100644 --- a/scripts/lib/lifecycle-column-census-baseline.json +++ b/scripts/lib/lifecycle-column-census-baseline.json @@ -20,7 +20,6 @@ "packages/engine/src/agent-heartbeat.ts": 4, "packages/engine/src/replan-target.ts": 4, "packages/engine/src/triage.ts": 4, - "packages/engine/src/usage-limit-detector.ts": 4, "packages/core/src/async-mission-store-queries.ts": 3, "packages/core/src/task-priority.ts": 3, "packages/core/src/task-store/async-merge-coordination.ts": 3, @@ -131,6 +130,8 @@ "packages/dashboard/app/components/TaskDetailModal.tsx\u0000triage": 2, "packages/engine/src/scheduler.ts\u0000in-progress": 2, "packages/engine/src/scheduler.ts\u0000in-review": 2, + "packages/engine/src/usage-limit-detector.ts\u0000archived": 2, + "packages/engine/src/usage-limit-detector.ts\u0000done": 2, "packages/cli/src/commands/task.ts\u0000archived": 1, "packages/cli/src/commands/task.ts\u0000done": 1, "packages/cli/src/extension.ts\u0000archived": 1,