batch(docs): fold the three workflow-learnings / annotation PRs into one (#2942)
## Family batch — replaces #2926, #2892, #2887 Per the consolidation directive: the u9/e2e **docs family**, folded into one branch and one CI run. Three PRs, five commits, **five files, comment and markdown only**. | folded PR | commits | |---|---| | #2892 `docs/union-vs-per-task` | the project union and the per-task answer are not ranked; date correction | | #2926 `docs/date-my-measured-claims` | date the measured claims (one was wrong); date the grep-vs-AST measurement in the SQL gate header | | #2887 `docs/archived-state-literals` | mark the three archived STATE literals as deliberate | Cherry-picked in original order with authorship preserved; all five applied clean, no conflicts. ## Scope is provably comment-only ``` docs/solutions/workflow-learnings/lifecycle-conversions-that-score-as-wins.md docs/solutions/workflow-learnings/project-union-versus-per-task-lanes.md packages/core/src/task-store/async-maintenance.ts ← FNXC DELIBERATE-LITERAL annotation packages/core/src/task-store/workflow-definitions.ts ← FNXC DELIBERATE-LITERAL annotation scripts/check-sql-column-literals.mjs ← header prose only ``` Every added line in `packages/` and `scripts/` is inside a comment — checked by filtering the diff for declarations, conditionals and returns, which returns nothing. The two core files gain `DELIBERATE-LITERAL` markers explaining that `'archived'` is a **state** marker there, not a lane: the sweep collects rows Fusion itself archived or soft-deleted, so widening to the resolved archived set would pull live cards into a cleanup pass. ## Verification (scoped, per the directive — not the full suite) - `pnpm lint` — clean - `check-sql-column-literals` — exit 0 (the file it annotates) - `check:lifecycle-columns` — exit 0 (the markers it adds are census-visible) - `sync-workflow-ir-callsite-allowlist.test.ts` — 3/3 ## A correction worth recording Mid-fold I saw a changeset, `self-healing.ts` and a test file in `git diff origin/main..HEAD` and nearly reported the batch as impure. They were **main's own commits** — `origin/main` advanced between branch creation and the diff, so the comparison was against a stale base. Rebasing onto current `main` reduced it to the five files above. Worth flagging for anyone else folding a family today: with `main` moving this fast, diff the branch **after** rebasing or the file list will lie to you. ## Closing the originals #2926, #2892 and #2887 are superseded by this and are being closed. I hold no PRs of my own in this family — all mine merged — so this fold is on behalf of the family rather than a rollup of my own work. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
@@ -152,7 +152,8 @@ hands in.** If your fixture passes the lane name, you have tested the consumer.
|
||||
computes that argument in production, and whether they compute it or spell it.
|
||||
|
||||
MEASURED, so nobody builds the wrong instrument: an AST probe for a call argument
|
||||
`{ column: "<legacy id>" }` finds **79 sites** across `packages/`. It flags the two real `moves.ts`
|
||||
`{ column: "<legacy id>" }` found **79 sites** across `packages/` when measured on 2026-07-30 (42 as
|
||||
of 2026-07-31, after the analytics conversions landed — the ratio is the point, not the total). It flags the two real `moves.ts`
|
||||
offenders, but most of the rest are legitimate — `set({ column: "archived" })` writing the archive
|
||||
state, `listTasks({ column: "todo" })` filtering a query. A blocking gate on this shape needs a
|
||||
curated list of consumers that interpret the column as a ROLE (as opposed to storing or filtering
|
||||
@@ -167,7 +168,8 @@ a plausible "the census cannot see this" theory; each was measured.
|
||||
**Membership tests with inline literals** — `["done","archived"].includes(task.column)`. The census
|
||||
counts binary comparisons only, so this shape would be invisible. **Zero sites.** Nobody writes it.
|
||||
|
||||
**Named legacy-id collections** — `const X = ["done","archived"]` then `X.has(col)`. **48 declarations**,
|
||||
**Named legacy-id collections** — `const X = ["done","archived"]` then `X.has(col)`. **48 declarations**
|
||||
measured 2026-07-30, 49 on 2026-07-31;
|
||||
and the raw count is misleading: on inspection they are all either
|
||||
|
||||
- the intentional fallback vocabulary the role helpers degrade to (`LEGACY_TERMINAL_COLUMNS`,
|
||||
@@ -182,9 +184,16 @@ would have been the same mistake as gating on `{ column: "<literal>" }` call arg
|
||||
mostly legitimate): a number that looks like a work list and is not.
|
||||
|
||||
**The rule:** measure a candidate surface, then inspect a sample, before reporting it OR instrumenting
|
||||
it. A raw count is a hypothesis. The SQL surface survived this test — 14 sites, all genuinely
|
||||
vocabulary-bound — and got a ratchet. These two did not, and got nothing, which is the correct
|
||||
outcome and cost an hour to establish rather than a wrong gate to maintain.
|
||||
it. A raw count is a hypothesis. The SQL surface survived this test and got a ratchet; these two did
|
||||
not, and got nothing, which is the correct outcome and cost an hour to establish rather than a wrong
|
||||
gate to maintain.
|
||||
|
||||
CORRECTION, 2026-07-31. This paragraph used to read "the SQL surface survived this test — 14 sites,
|
||||
all genuinely vocabulary-bound". Both halves were wrong within a day. The population was 31 once the
|
||||
scanner stopped missing Drizzle-templated queries, and hand-reading split it into roughly fourteen
|
||||
lane-bound sites, eleven `archived` comparisons (three of them CORRECT as literals — they read a
|
||||
state, not a lane), and one in dead code. Writing "all genuinely vocabulary-bound" was the same
|
||||
mistake this section warns about, committed in the sentence that warns about it.
|
||||
|
||||
## Guards start catching other people's work, not just yours
|
||||
|
||||
|
||||
@@ -0,0 +1,80 @@
|
||||
---
|
||||
category: workflow-learnings
|
||||
module: packages/core/src/project-lane-vocabulary
|
||||
tags: [lifecycle-columns, resolved-lanes, arity, project-union, per-task]
|
||||
problem_type: design-decision
|
||||
applies_when: choosing between resolveProjectColumnsForRoles and a per-task lane resolution
|
||||
---
|
||||
|
||||
# The project union and the per-task answer are not ranked
|
||||
|
||||
Recorded 2026-07-30, after the same reviewer finding arrived on five PRs in one day and got **four
|
||||
different correct answers**. The instinct the program had built by then — "a union is the lazy option,
|
||||
resolve per task" — is wrong often enough to be worth writing down, because acting on it shipped a
|
||||
regression.
|
||||
|
||||
`resolveProjectColumnsForRoles(store, roles)` returns every column any workflow in the project gives
|
||||
the role, unioned with the legacy ids. A per-task resolution answers for one card's own board. Neither
|
||||
is the better tool.
|
||||
|
||||
## The question that decides it
|
||||
|
||||
Not "can I resolve per task?" — usually you can. It is:
|
||||
|
||||
> **What does over-inclusion cost here, and what does a missing selection cost?**
|
||||
|
||||
Those are the two failure directions, and they are asymmetric in a way that changes per call site.
|
||||
|
||||
## The four answers, with what made each one right
|
||||
|
||||
| site | answer | why |
|
||||
|---|---|---|
|
||||
| `awaitingPlanning` board enrichment | **union** | over-inclusion costs one extra file read; the per-card predicate still answers per card, so a widened candidate set cannot produce a wrong badge |
|
||||
| glasses completion notifier | **narrow** | a notification is an outward action with no downstream narrowing — over-inclusion is a wrong notification reaching a user |
|
||||
| `workflow-analytics` completions | **narrow, feasible** | the rows carry a workflow id (`GROUP BY`), so the keyed answer was available and the union was simply the wrong arity |
|
||||
| `team-analytics` totals | **union, accepted** | per-task means one workflow read per task and moving an indexed `COUNT` into JS; the cost is a number an operator reads, recorded rather than paid |
|
||||
| archived-document guards | **union, after trying narrow** | see below |
|
||||
|
||||
## The one that reversed the instinct
|
||||
|
||||
`comments-ops.ts` guards whether a card's documents are read-only. Both callers have `taskId`, so the
|
||||
keyed answer looked available and the arity argument said take it.
|
||||
|
||||
Switching to `resolveWorkflowIrForTask` **broke a passing renamed-lane test**: a card in a renamed
|
||||
archived lane stopped being read-only. The per-task resolver needs the task's own workflow
|
||||
**selection**, and with none recorded it degrades to the built-in IR, whose archived lane is
|
||||
`archived`. The project union needs no selection — which is exactly why it caught the renamed card.
|
||||
|
||||
The failure modes were not the same size:
|
||||
|
||||
- **union**: a live card loses document writes, and only where two workflows reuse one id with
|
||||
different traits — a configuration nobody has reported.
|
||||
- **per-task**: every renamed-lane card with no recorded selection silently becomes *writable while
|
||||
archived* — the defect the guard exists to prevent, with a test proving it.
|
||||
|
||||
Reverted. The correct fix is **per-task with a union fallback when the selection is absent** — narrow
|
||||
when the card can answer, broad when it cannot — which needs "no selection" distinguished from
|
||||
"selection resolved to the default", i.e. the provenance form.
|
||||
|
||||
## Rules that fall out of it
|
||||
|
||||
1. **A union is not a shortcut.** It is the right answer wherever a card cannot be assumed to have a
|
||||
resolvable workflow, because it does not depend on one.
|
||||
2. **Ask what happens with no selection before converting.** `resolveWorkflowIrForTask` does not fail,
|
||||
it *substitutes* the built-in board — so a per-task conversion silently answers with the wrong
|
||||
vocabulary exactly where the renamed-board bug lives.
|
||||
3. **Over-inclusion is free for a read, costly for an action, and misleading for a metric.** Query,
|
||||
candidate filter, anything narrowed downstream: union. Notification, move, write gate: narrow.
|
||||
Aggregate shown to a human: narrow if the rows are keyed, otherwise accept and record.
|
||||
4. **Swapping one defect for a bigger one drops a guard count and looks like progress.** That is the
|
||||
failure mode `lifecycle-conversions-that-score-as-wins.md` catalogues; this is the same trap
|
||||
reached through a correct-sounding principle rather than a careless edit.
|
||||
|
||||
## Also unresolved, and related
|
||||
|
||||
`resolveProjectColumnsForRoles` seeds only the legacy ids for a project whose workflows declare **no**
|
||||
lifecycle traits at all, so a hand-authored v2 board with renamed columns and no traits contributes
|
||||
nothing and its cards are invisible to every sweep that queries by role. The three-state rule at
|
||||
project scope. The fix cannot be a changed default for the reason above — sweeps want the widening and
|
||||
the aggregators do not — so it wants an opt-in
|
||||
(`{ untraitedProject: "declared-columns" }`). Recorded at three call sites in `self-healing.ts`.
|
||||
@@ -37,6 +37,13 @@ export async function pruneAgentLogFilesAsync(
|
||||
severityAuditLog.warn("[fusion] PostgreSQL agent-log-file pruning is using the legacy unscoped project sentinel because asyncLayer.projectId is missing");
|
||||
}
|
||||
const projectId = boundProjectId || "__legacy_unscoped__";
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-22:12 DELIBERATE-LITERAL:
|
||||
`'archived'` is the STATE marker here, not a lane. This sweep collects rows Fusion itself archived
|
||||
or soft-deleted; a card merely sitting in a workflow's archived-TRAIT lane is live work and must not
|
||||
be collected. Widening to the resolved archived set would pull real cards into a cleanup pass.
|
||||
See #2839 for why the live-VIEW exclusions that share this literal are a different question.
|
||||
*/
|
||||
const rows = (await layer.db.execute(
|
||||
sql`SELECT id FROM project.tasks WHERE project_id = ${projectId} AND (deleted_at IS NOT NULL OR "column" = 'archived')`,
|
||||
)) as unknown as Array<{ id: string }>;
|
||||
|
||||
@@ -117,6 +117,20 @@ export function migrateLegacyArchiveEntriesToArchiveDbImpl(store: TaskStore): vo
|
||||
}
|
||||
|
||||
export async function migrateActiveArchivedTasksToArchiveDbImpl(store: TaskStore): Promise<void> {
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-22:10 DELIBERATE-LITERAL:
|
||||
`'archived'` here is the STATE marker, not a board lane, and must NOT be widened to the resolved
|
||||
archived columns.
|
||||
|
||||
This finds live rows that Fusion's own archive path stamped (`archive-lifecycle-2.ts` and
|
||||
`serialization.ts` both hardcode `column: "archived"`) so they can be migrated into the archive
|
||||
DB. A workflow may also declare an archived-TRAIT lane under any id — `resolveLifecycleColumns`
|
||||
resolves it, and a card can be moved there — but such a card was never archived by Fusion, has no
|
||||
archive-store row, and migrating it would move live work out of the board.
|
||||
|
||||
So the resolved set is the wrong question at this site even though it is the right one for the
|
||||
live-view exclusions that share this literal. See issue #2839 for the split.
|
||||
*/
|
||||
const rows = store.db.prepare(`SELECT * FROM tasks WHERE "column" = 'archived'`).all() as unknown as TaskRow[];
|
||||
if (rows.length === 0) {
|
||||
return;
|
||||
@@ -906,6 +920,12 @@ export function pruneAgentLogFilesImpl(store: TaskStore, retentionDays: number):
|
||||
if (!Number.isFinite(retentionDays) || retentionDays <= 0) {
|
||||
return { prunedFiles: 0, prunedEntries: 0, freedBytes: 0 };
|
||||
}
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-22:14 DELIBERATE-LITERAL:
|
||||
STATE marker again, same reasoning as migrateActiveArchivedTasksToArchiveDbImpl above: this prunes
|
||||
agent-log files for rows Fusion archived or soft-deleted. A card in a workflow's archived-TRAIT
|
||||
lane is live work whose logs must survive, so the resolved set would delete data here.
|
||||
*/
|
||||
// Only prune JSONL files for tasks that are no longer active (soft-deleted or archived)
|
||||
const inactiveTaskIds = new Set(
|
||||
(
|
||||
|
||||
@@ -23,8 +23,10 @@ baseline records per-file counts, a new file or a higher count fails, and a LOWE
|
||||
the baseline is ratcheted down as sites are migrated rather than silently drifting.
|
||||
|
||||
COMMENTS ARE NOT MATCHED, and that is the whole reason this is AST-based. A line-oriented grep for
|
||||
the same pattern reports 37 hits, 25 of which are prose quoting `column === "done"` in an explanatory
|
||||
note. A guard with a 68% false-positive rate teaches its readers to skip it, and this repo already
|
||||
the same pattern reported 37 hits when this was written (2026-07-30), 25 of them prose quoting
|
||||
`column === "done"` in an explanatory note. Re-measured 2026-07-31: 38 grep hits against 20 real ones.
|
||||
The totals drift as conversions land and comments do not — the RATIO is the argument, and it has held
|
||||
at roughly half. A guard with a 68% false-positive rate teaches its readers to skip it, and this repo already
|
||||
learned that lesson the expensive way. Comments are not AST nodes, so walking string and template
|
||||
literals cannot match them at all.
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user