docs(engine): mark the usage-limit terminal filters DELIBERATE-LITERAL — a documented false positive that has now baited two workers (#2767)

## 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) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-30 08:31:47 -07:00
committed by GitHub
parent 2e4905fa0e
commit 7f3eee9db7
2 changed files with 24 additions and 1 deletions

View File

@@ -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"

View File

@@ -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,