diff --git a/.changeset/clever-sloths-dart.md b/.changeset/clever-sloths-dart.md new file mode 100644 index 000000000..369373fb7 --- /dev/null +++ b/.changeset/clever-sloths-dart.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 719 +--- +**Workflow size budget now measures bytes, not lines (#717).** `tests/workflow-size-budget.test.cjs` re-bases its tier ceilings (XL/LARGE/DEFAULT) from line counts to byte counts — deterministic, no tokenizer, and matching the unit vendors bound on (Codex's 32,768-byte project_doc_max_bytes cap). The #597 tighten-only ratchet and per-file semantics are unchanged; the budget's caching-independent quality rationale (context rot / attention budget) is now documented. diff --git a/CONTEXT.md b/CONTEXT.md index 153b0df58..9a228a50f 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -195,7 +195,7 @@ The canonical lint infrastructure adopted in ADR 452 (`docs/adr/452-eslint-lint- `RULESET.TESTS.eslint-harness=ADR 452 (2026-05-28): ESLint flat config + typescript-eslint + eslint-plugin-n + eslint-plugin-no-only-tests + local plugin at scripts/eslint-rules/; replaces scripts/lint-*.cjs regex scanners; three test-rigor rules (local/no-source-grep, local/no-magic-sleep-in-tests, local/no-elapsed-assertion) ship at warn, promoted to error after #453 cleanup sweep merges` `RULESET.WORKFLOW_MARKDOWN.FENCES=preserve opening language fence when editing shell snippets in workflow markdown; malformed fence creates fresh CR threads (MD040)` -`RULESET.WORKFLOW_SIZE_BUDGET=workflow-size-budget can fail otherwise-valid review fixes; XL workflows <=1800 lines or trim prose before final checks` +`RULESET.WORKFLOW_SIZE_BUDGET=workflow-size-budget (#717) measures BYTES not lines; tiers XL<=90000 / LARGE<=54000 / DEFAULT<=38000 bytes, discuss-phase<30000; can fail otherwise-valid review fixes — trim prose or extract LAZILY-loaded content (eager @-imports don't reduce loaded context) before final checks` `RULESET.WORKFLOW_FILE_NAMES=workflow files use hyphens; XML attributes must match (extract-learnings not extract_learnings); tests should pin exact hyphenated name` `RULESET.WORKFLOW_EXECUTION_CONTEXT=@-ref in commands/gsd/*.md must resolve to an existing file on disk; regression test in tests/bug-3135-capture-backlog-workflow.test.cjs; INVENTORY.md row + INVENTORY-MANIFEST.json families.workflows must stay in sync; "Invoked by" attribution must move when a flag absorbs a micro-skill` `RULESET.WORKFLOW_EXECUTE_END_TO_END=ADR-0002 standard for single-workflow commands is "Execute end-to-end." (no bolded **Follow the X workflow** fragments); flag-dispatch routing uses "execute the X workflow end-to-end." in routing bullets` @@ -238,7 +238,6 @@ The canonical lint infrastructure adopted in ADR 452 (`docs/adr/452-eslint-lint- `RULESET.CODERABBIT.GUARD.SCOPE=if a new @me open PR appears during final list, include it in the same guard pass before declaring all-open-PRs complete` `RULESET.TESTS.CODERABBIT_FIX=prefer exported-function behavioral tests over source-grep; lint-no-source-grep rejects readFileSync source assertions without allow-test-rule` `RULESET.WORKFLOW_MARKDOWN.FENCES=when editing shell snippets inside workflow markdown, preserve the opening language fence; malformed fence can create fresh CodeRabbit threads` -`RULESET.WORKFLOW_SIZE_BUDGET=workflow-size-budget can fail otherwise-valid review fixes; keep XL workflows <=1800 lines or trim prose in same PR before final checks` `RULESET.GEMINI.TOOLS.ask_user=Gemini CLI has no ask_user tool; filter both AskUserQuestion and lowercase ask_user from tools frontmatter and neutralize both names in Gemini body text` `RULESET.GEMINI.TEST_SENTINEL=convertClaudeToGeminiAgent regression should assert tools excludes ask_user, body excludes AskUserQuestion/ask_user, and Read still maps to read_file` diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 6b31ecc24..0443991e5 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -145,19 +145,43 @@ Orchestration logic that commands reference. Contains the step-by-step process i #### Progressive disclosure for workflows Workflow files are loaded verbatim into Claude's context every time the -corresponding `/gsd-*` command is invoked. To keep that cost bounded, the -workflow size budget enforced by `tests/workflow-size-budget.test.cjs` -mirrors the agent budget from #2361: +corresponding `/gsd-*` command is invoked. The workflow size budget enforced by +`tests/workflow-size-budget.test.cjs` keeps each file bounded, mirroring the +agent budget from #2361. The budget is measured in **bytes** (#717), not lines: +line count over-penalizes prose and under-catches token-dense tables and code +blocks, whereas bytes are deterministic and match the unit our vendors bound on +— Codex truncates instruction docs past 32,768 bytes (`project_doc_max_bytes`). +We adopt that unit, not that exact number: the XL/LARGE ceilings below sit above +32,768 because these are grandfathered top-level orchestrators loaded by Claude, +not Codex AGENTS.md docs. -| Tier | Per-file line limit | -|-----------|--------------------| -| `XL` | 1700 — top-level orchestrators (`execute-phase`, `plan-phase`, `new-project`) | -| `LARGE` | 1500 — multi-step planners and large feature workflows | -| `DEFAULT` | 1000 — focused single-purpose workflows (the target tier) | +| Tier | Per-file byte limit | +|-----------|---------------------| +| `XL` | 90,000 — top-level orchestrators (`execute-phase`, `plan-phase`, `new-project`) | +| `LARGE` | 54,000 — multi-step planners and large feature workflows | +| `DEFAULT` | 38,000 — focused single-purpose workflows (the target tier) | -`workflows/discuss-phase.md` is held to a stricter <500-line ceiling per -issue #2551. When a workflow grows beyond its tier, extract per-mode bodies -into `workflows//modes/.md`, templates into +Ceilings are not fixed forever: under the tighten-only ratchet (#597) each one +tracks its tier's current high-water mark within a small grace band, so budgets +may only decrease over time. + +**Why the budget exists.** With prompt caching the per-invocation *cost* of a +large workflow is modest (cache reads run ~10% of input). The stronger, +caching-independent reason is **quality**: as context grows, recall and +reasoning degrade ("context rot" / attention budget), so leaner, higher-signal +instructions produce better plans. The ceiling protects the agent's attention, +not just the token bill. + +Because the budget measures one file, it is a proxy for the real goal — +*bounded loaded context*. Extraction only helps when the extracted content is +loaded **lazily** (Read at the step that needs it). Moving prose into a file +that is still eagerly `@`-imported shrinks the measured file without shrinking +loaded context, which games the proxy rather than serving the goal. + +`workflows/discuss-phase.md` is held to a stricter <30,000-byte ceiling per +issue #2551 (originally <500 lines; re-based to bytes for #717). When a workflow grows +beyond its tier, extract per-mode bodies into +`workflows//modes/.md`, templates into `workflows//templates/`, and shared knowledge into `gsd-core/references/`. The parent file becomes a thin dispatcher that Reads only the mode and template files needed for the current invocation. diff --git a/tests/feat-3039-help-tiered.test.cjs b/tests/feat-3039-help-tiered.test.cjs index 4c981f45c..e97d2e8ee 100644 --- a/tests/feat-3039-help-tiered.test.cjs +++ b/tests/feat-3039-help-tiered.test.cjs @@ -58,7 +58,8 @@ const MODE_FILES = ['brief.md', 'default.md', 'full.md', 'topic.md']; const BRIEF_BUDGET = 30; // DEFAULT ceiling lowered from 70 → 60 (actualMax=50; #597 ratchet-down). const DEFAULT_BUDGET = 60; -// full.md is the LARGE tier (see workflow-size-budget.test.cjs — LARGE_BUDGET = 1500). +// full.md is the LARGE tier (see workflow-size-budget.test.cjs — now byte-based per #717; +// this FULL_BUDGET is a separate line-count budget for help/modes/full.md). // The size-budget test is non-recursive so full.md is not covered there; cap it here. // FULL ceiling lowered from 1500 → 844 (actualMax=784; #597 ratchet-down). const FULL_BUDGET = 844; diff --git a/tests/workflow-size-budget.test.cjs b/tests/workflow-size-budget.test.cjs index dab96d718..65da82e37 100644 --- a/tests/workflow-size-budget.test.cjs +++ b/tests/workflow-size-budget.test.cjs @@ -1,33 +1,64 @@ // allow-test-rule: source-text-is-the-product -// Tests measure line counts of workflow files — the workflow file text IS the +// Tests measure byte sizes of workflow files — the workflow file text IS the // product loaded by agents at runtime. No command output is parsed. // Migrated from pending-migration-to-typed-ir per #455. /** - * Workflow size budget. + * Workflow size budget (measured in BYTES — see #717). * * Workflow definitions in `gsd-core/workflows/*.md` are loaded verbatim - * into Claude's context every time the corresponding `/gsd:*` command is + * into the agent's context every time the corresponding `/gsd:*` command is * invoked. Unbounded growth is paid on every invocation across every session. * + * ## Why bytes, not lines (#717) + * + * Line count is a poor proxy: markdown tables and fenced code blocks are + * token-dense, so a line budget over-penalizes prose and under-catches dense + * additions. Bytes are cheap, deterministic, and need no tokenizer. They are + * also the UNIT our vendors bound on — Codex caps instruction docs at 32,768 + * bytes (`project_doc_max_bytes`) and truncates past it. We adopt that unit, + * not that exact number: our XL/LARGE ceilings sit above 32,768 because these + * are grandfathered top-level orchestrators loaded by Claude, not Codex + * AGENTS.md docs — the goal is a bounded, ratcheting budget, not Codex parity. + * + * ## Why the budget exists at all (the quality argument, not just cost) + * + * With prompt caching the per-invocation *cost* premise is weak (cache reads + * are ~10% of input). The stronger, caching-independent reason is QUALITY: + * larger context degrades recall and reasoning ("context rot" / attention + * budget). Lean, high-signal instructions produce better plans. The ceiling + * protects the agent's attention, not just the token bill. + * + * ## The goal this metric is a proxy for (read before gaming it — #717) + * + * The real target is bounded *loaded* context. This test measures one file's + * bytes, but `@~/.claude/gsd-core/references/...` imports are loaded EAGERLY + * into context. Moving prose into an eagerly @-imported reference shrinks the + * measured file while leaving (or growing) total loaded context — that is + * gaming the proxy, not improving the goal. Legitimate extraction is LAZY: + * content Read only at the step that needs it (see the discuss-phase mode/ + * template tests below, which forbid templates in ). + * * Tiered the same way as agent budgets (#2361): - * - XL : top-level orchestrators (e.g., execute-phase, autonomous) + * - XL : top-level orchestrators (e.g., execute-phase, plan-phase) * - LARGE : multi-step planners * - DEFAULT : focused single-purpose workflows (target tier) * * Raising a budget is a deliberate choice — adjust the constant, write a * rationale in the PR, and confirm the bloat is not duplicated content - * that belongs in `gsd-core/references/` or a per-mode subdirectory - * (see `workflows/discuss-phase/modes/` for the progressive-disclosure - * pattern introduced by #2551). + * that belongs in `gsd-core/references/` (lazily loaded) or a per-mode + * subdirectory (see `workflows/discuss-phase/modes/`, #2551). * * Tighten-only invariant (issue #597): ceilings track the tier high-water mark - * within GRACE lines. Budgets may only decrease, never silently creep upward. + * within GRACE bytes. Budgets may only decrease, never silently creep upward. * The assertTightCeiling() call below enforces this automatically. * * See: + * - https://github.com/open-gsd/gsd-core/issues/717 (bytes re-base + rationale) * - https://github.com/open-gsd/gsd-core/issues/2551 (this test) * - https://github.com/open-gsd/gsd-core/issues/2361 (agent budget) + * - https://developers.openai.com/codex/guides/agents-md (Codex 32 KB cap) + * - https://www.anthropic.com/engineering/effective-context-engineering-for-ai-agents */ const { test, describe } = require('node:test'); @@ -38,46 +69,45 @@ const { assertTightCeiling } = require('../scripts/lib/allowlist-ratchet.cjs'); const WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows'); -// Grace band: maximum allowed slack (ceiling − actualMax) before a ceiling is -// considered too loose. 60 lines gives one reasonable screen of breathing room -// without permitting gross inflation. -const GRACE = 60; +// Grace band: maximum allowed slack (ceiling − actualMax) in BYTES before a +// ceiling is considered too loose. 3000 bytes ≈ the prior 60-line grace +// re-expressed for the #717 unit swap (these files run ~36–50 bytes/line, so +// ~60–80 lines of breathing room) without permitting gross inflation. +const GRACE = 3000; -// Bumped from 1700 → 1800 in #3181 to absorb MVP-mode verb-call additions -// in execute-phase.md (1727 → ) and plan-phase.md (1714 → ) from #3178. -// Follow-up #3182 (TBD): extract MVP-mode bodies to `/modes/mvp.md` -// per the discuss-phase/modes/ precedent and revert this back to 1700. -// Bumped from 1800 → 1810 in #3707 to absorb the startup orphan-sweep -// block added to execute-phase.md (+2 lines: one comment + one bash command). -// XL ceiling kept at 1810 (actualMax=1810, plan-phase; slack=0 ≤ GRACE=60). -const XL_BUDGET = 1810; -// LARGE ceiling lowered from 1500 → 1236 (actualMax=1176, docs-update; #597 ratchet-down). -const LARGE_BUDGET = 1236; -// DEFAULT ceiling lowered from 1000 → 870 (actualMax=810, settings-advanced; #597 ratchet-down). -const DEFAULT_BUDGET = 870; +// Byte ceilings (#717 re-base from lines). Each tier's ceiling tracks the +// current high-water mark within GRACE (#597 tighten-only ratchet). +// XL high-water mark is execute-phase.md — note that under LINES it was +// plan-phase; bytes genuinely re-rank the tier, which is the point of #717. +// actualMax=87005 (execute-phase); slack=2995 ≤ GRACE. +const XL_BUDGET = 90000; +// LARGE high-water mark is docs-update.md. actualMax=51184; slack=2816 ≤ GRACE. +const LARGE_BUDGET = 54000; +// DEFAULT high-water mark is settings-advanced.md. actualMax=35183; slack=2817 ≤ GRACE. +const DEFAULT_BUDGET = 38000; // Top-level orchestrators that own end-to-end multi-phase rubrics. -// Grandfathered at current sizes — see PR #2551 for #2551 progressive-disclosure -// pattern that future shrinks should follow. +// Grandfathered at current sizes — see PR #2551 for the progressive-disclosure +// pattern that future shrinks should follow. Byte counts noted for reference. const XL_WORKFLOWS = new Set([ - 'execute-phase', // 1727 (post-MVP-verb-integration; was 1622) - 'plan-phase', // 1714 (post-MVP-verb-integration; was 1493) - 'new-project', // 1391 + 'execute-phase', // 87005 bytes (tier high-water mark) + 'plan-phase', // 85068 bytes + 'new-project', // 55850 bytes ]); // Multi-step planners and bigger feature workflows. Grandfathered. const LARGE_WORKFLOWS = new Set([ - 'docs-update', // 1155 - 'autonomous', // 789 - 'complete-milestone', // 847 - 'verify-work', // 740 - 'transition', // 693 - 'discuss-phase-assumptions', // 670 - 'progress', // 619 - 'new-milestone', // 611 - 'update', // 587 - 'quick', // 971 - 'code-review', // 515 + 'docs-update', // 51184 bytes (tier high-water mark) + 'autonomous', // 32655 + 'complete-milestone', // 26284 + 'verify-work', // 26896 + 'transition', // 18201 + 'discuss-phase-assumptions', // 23398 + 'progress', // 23061 + 'new-milestone', // 26582 + 'update', // 19334 + 'quick', // 42484 + 'code-review', // 25500 ]); const ALL_WORKFLOWS = fs.readdirSync(WORKFLOWS_DIR) @@ -90,38 +120,40 @@ function budgetFor(workflow) { return { tier: 'DEFAULT', limit: DEFAULT_BUDGET }; } -function lineCount(filePath) { - const content = fs.readFileSync(filePath, 'utf-8'); - if (content.length === 0) return 0; - const trailingNewline = content.endsWith('\n') ? 1 : 0; - return content.split('\n').length - trailingNewline; +function byteCount(filePath) { + // Match `wc -c`: count every byte on disk, including any trailing newline. + // Deliberately NOT the trailing-newline-stripping logic the old lineCount() + // used — the byte ceilings are calibrated against raw `wc -c` output. + return fs.statSync(filePath).size; } -describe('SIZE: workflow line-count budget', () => { +describe('SIZE: workflow byte-size budget', () => { for (const workflow of ALL_WORKFLOWS) { const { tier, limit } = budgetFor(workflow); - test(`${workflow} (${tier}) stays under ${limit} lines`, () => { + test(`${workflow} (${tier}) stays under ${limit} bytes`, () => { const filePath = path.join(WORKFLOWS_DIR, workflow + '.md'); - const lines = lineCount(filePath); + const bytes = byteCount(filePath); assert.ok( - lines <= limit, - `${workflow}.md has ${lines} lines — exceeds ${tier} budget of ${limit}. ` + + bytes <= limit, + `${workflow}.md is ${bytes} bytes — exceeds ${tier} budget of ${limit}. ` + `Extract per-mode bodies to a workflows/${workflow}/modes/ subdirectory, ` + `templates to workflows/${workflow}/templates/, or shared references ` + - `to gsd-core/references/. See workflows/discuss-phase/ for the pattern.` + `to gsd-core/references/ — and load them LAZILY (not via @-required_reading, ` + + `which would shrink this file's bytes without shrinking loaded context). ` + + `See workflows/discuss-phase/ for the pattern.` ); }); } }); describe('SIZE: tier anti-creep (tighten-only ceilings, issue #597)', () => { - // For each tier, compute the high-water mark across all files in that tier - // and assert the ceiling stays tight. Prevents budgets from silently drifting - // upward: ceiling − actualMax must not exceed GRACE. + // For each tier, compute the high-water mark (in bytes) across all files in + // that tier and assert the ceiling stays tight. Prevents budgets from + // silently drifting upward: ceiling − actualMax must not exceed GRACE. test('XL tier: ceiling tracks high-water mark within GRACE', () => { const values = ALL_WORKFLOWS .filter(w => XL_WORKFLOWS.has(w)) - .map(w => lineCount(path.join(WORKFLOWS_DIR, w + '.md'))); + .map(w => byteCount(path.join(WORKFLOWS_DIR, w + '.md'))); const actualMax = Math.max(...values); assertTightCeiling({ label: 'XL', actualMax, ceiling: XL_BUDGET, grace: GRACE, fail: assert.fail }); }); @@ -129,7 +161,7 @@ describe('SIZE: tier anti-creep (tighten-only ceilings, issue #597)', () => { test('LARGE tier: ceiling tracks high-water mark within GRACE', () => { const values = ALL_WORKFLOWS .filter(w => LARGE_WORKFLOWS.has(w)) - .map(w => lineCount(path.join(WORKFLOWS_DIR, w + '.md'))); + .map(w => byteCount(path.join(WORKFLOWS_DIR, w + '.md'))); const actualMax = Math.max(...values); assertTightCeiling({ label: 'LARGE', actualMax, ceiling: LARGE_BUDGET, grace: GRACE, fail: assert.fail }); }); @@ -137,24 +169,27 @@ describe('SIZE: tier anti-creep (tighten-only ceilings, issue #597)', () => { test('DEFAULT tier: ceiling tracks high-water mark within GRACE', () => { const values = ALL_WORKFLOWS .filter(w => !XL_WORKFLOWS.has(w) && !LARGE_WORKFLOWS.has(w)) - .map(w => lineCount(path.join(WORKFLOWS_DIR, w + '.md'))); + .map(w => byteCount(path.join(WORKFLOWS_DIR, w + '.md'))); const actualMax = Math.max(...values); assertTightCeiling({ label: 'DEFAULT', actualMax, ceiling: DEFAULT_BUDGET, grace: GRACE, fail: assert.fail }); }); }); describe('SIZE: discuss-phase progressive disclosure (issue #2551)', () => { - // Issue #2551 explicitly targets discuss-phase.md at <500 lines, separate from - // the per-tier grandfathered budgets above. This is the headline metric of the - // refactor — every other workflow above 500 is grandfathered at its current - // size and may shrink later by following the same pattern. - const DISCUSS_PHASE_TARGET = 500; - test(`discuss-phase.md is under ${DISCUSS_PHASE_TARGET} lines (issue #2551 target)`, () => { + // Issue #2551 targets discuss-phase.md as a thin dispatcher, separate from + // the per-tier grandfathered budgets above. Originally expressed as <500 + // lines; re-based to bytes for #717 (500 lines ≈ 28 KB at these files' + // density; set to 30 KB to preserve the thin-dispatcher intent with modest + // headroom). This is the headline metric of the refactor — every other + // workflow above its tier is grandfathered and may shrink later via the + // same pattern. + const DISCUSS_PHASE_TARGET = 30000; + test(`discuss-phase.md is under ${DISCUSS_PHASE_TARGET} bytes (issue #2551 target)`, () => { const filePath = path.join(WORKFLOWS_DIR, 'discuss-phase.md'); - const lines = lineCount(filePath); + const bytes = byteCount(filePath); assert.ok( - lines < DISCUSS_PHASE_TARGET, - `discuss-phase.md has ${lines} lines — must be under ${DISCUSS_PHASE_TARGET} per #2551. ` + + bytes < DISCUSS_PHASE_TARGET, + `discuss-phase.md is ${bytes} bytes — must be under ${DISCUSS_PHASE_TARGET} per #2551. ` + `Per-mode logic belongs in workflows/discuss-phase/modes/.md, ` + `templates in workflows/discuss-phase/templates/.` );