mirror of
https://github.com/glittercowboy/get-shit-done
synced 2026-04-25 17:25:23 +02:00
fix(#2549,#2550,#2552): bound discuss-phase context reads, add phase-type map selection, prohibit split reads (#2590)
#2549: load_prior_context was reading every prior *-CONTEXT.md file, growing linearly with project phase count. Cap to the 3 most recent phases. If .planning/DECISIONS-INDEX.md exists, read that instead. #2550: scout_codebase claimed to select maps "based on phase type" but had no classifier — agents read all 7 maps. Replace with an explicit phase-type-to-maps table (2–3 maps per phase type) with a Mixed fallback. #2552: Add explicit instruction not to split-read the same file at two different offsets. Split reads break prompt cache reuse and cost more than a single full read. Closes #2549 Closes #2550 Closes #2552 Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -318,13 +318,20 @@ Extract from these:
|
|||||||
- **REQUIREMENTS.md** — Acceptance criteria, constraints, must-haves vs nice-to-haves
|
- **REQUIREMENTS.md** — Acceptance criteria, constraints, must-haves vs nice-to-haves
|
||||||
- **STATE.md** — Current progress, any flags or session notes
|
- **STATE.md** — Current progress, any flags or session notes
|
||||||
|
|
||||||
**Step 2: Read all prior CONTEXT.md files**
|
**Step 2: Read bounded prior CONTEXT.md files**
|
||||||
|
|
||||||
|
Reading every prior CONTEXT.md grows linearly with phase count and inflates context cost. Instead, read a bounded set:
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
# Find all CONTEXT.md files from phases before current
|
# Find CONTEXT.md files from phases before current, sorted newest-first
|
||||||
(find .planning/phases -name "*-CONTEXT.md" 2>/dev/null || true) | sort
|
(find .planning/phases -name "*-CONTEXT.md" 2>/dev/null || true) | sort -r
|
||||||
```
|
```
|
||||||
|
|
||||||
For each CONTEXT.md where phase number < current phase:
|
Read at most **3** prior CONTEXT.md files (the most recent 3 phases before the current one). This is sufficient for decision continuity — earlier decisions are already captured in PROJECT.md and REQUIREMENTS.md.
|
||||||
|
|
||||||
|
If `.planning/DECISIONS-INDEX.md` exists, read that instead of individual CONTEXT files — it is a bounded rolling summary that supersedes per-phase reads.
|
||||||
|
|
||||||
|
For each CONTEXT.md read:
|
||||||
- Read the `<decisions>` section — these are locked preferences
|
- Read the `<decisions>` section — these are locked preferences
|
||||||
- Read `<specifics>` — particular references or "I want it like X" moments
|
- Read `<specifics>` — particular references or "I want it like X" moments
|
||||||
- Note any patterns (e.g., "user consistently prefers minimal UI", "user rejected single-key shortcuts")
|
- Note any patterns (e.g., "user consistently prefers minimal UI", "user rejected single-key shortcuts")
|
||||||
@@ -436,7 +443,23 @@ Lightweight scan of existing code to inform gray area identification and discuss
|
|||||||
ls .planning/codebase/*.md 2>/dev/null || true
|
ls .planning/codebase/*.md 2>/dev/null || true
|
||||||
```
|
```
|
||||||
|
|
||||||
**If codebase maps exist:** Read the most relevant ones (CONVENTIONS.md, STRUCTURE.md, STACK.md based on phase type). Extract:
|
**If codebase maps exist:** Select 2–3 maps based on phase type. Do NOT read all seven — that inflates context without improving discussion quality. Use this selection table:
|
||||||
|
|
||||||
|
| Phase type (infer from title + ROADMAP entry) | Read these maps |
|
||||||
|
|---|---|
|
||||||
|
| UI / frontend / styling / design | CONVENTIONS.md, STRUCTURE.md, STACK.md |
|
||||||
|
| Backend / API / service / data model | STACK.md, ARCHITECTURE.md, INTEGRATIONS.md |
|
||||||
|
| Integration / third-party / provider | STACK.md, INTEGRATIONS.md, ARCHITECTURE.md |
|
||||||
|
| Infrastructure / DevOps / CI / deploy | STACK.md, ARCHITECTURE.md, INTEGRATIONS.md |
|
||||||
|
| Testing / QA / coverage | TESTING.md, CONVENTIONS.md, STRUCTURE.md |
|
||||||
|
| Documentation / content | CONVENTIONS.md, STRUCTURE.md |
|
||||||
|
| Mixed / unclear | STACK.md, ARCHITECTURE.md, CONVENTIONS.md |
|
||||||
|
|
||||||
|
Read CONCERNS.md only if the phase explicitly addresses known concerns or security issues.
|
||||||
|
|
||||||
|
**Important — no split reads:** Read each map file in a single Read call. Do not read the same file at two different offsets — split reads break prompt cache reuse and cost more than a single full read.
|
||||||
|
|
||||||
|
Extract:
|
||||||
- Reusable components/hooks/utilities
|
- Reusable components/hooks/utilities
|
||||||
- Established patterns (state management, styling, data fetching)
|
- Established patterns (state management, styling, data fetching)
|
||||||
- Integration points (where new code would connect)
|
- Integration points (where new code would connect)
|
||||||
|
|||||||
78
tests/bug-2549-2550-2552-discuss-phase-context.test.cjs
Normal file
78
tests/bug-2549-2550-2552-discuss-phase-context.test.cjs
Normal file
@@ -0,0 +1,78 @@
|
|||||||
|
'use strict';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Bugs #2549, #2550, #2552: discuss-phase context bloat and cache invalidation.
|
||||||
|
*
|
||||||
|
* #2549: load_prior_context must cap prior CONTEXT.md reads (was O(phases))
|
||||||
|
* #2550: scout_codebase must select maps by phase type (was always all 7)
|
||||||
|
* #2552: scout_codebase must not instruct split reads of the same file
|
||||||
|
*/
|
||||||
|
|
||||||
|
const { test, describe } = require('node:test');
|
||||||
|
const assert = require('node:assert/strict');
|
||||||
|
const fs = require('node:fs');
|
||||||
|
const path = require('node:path');
|
||||||
|
|
||||||
|
const DISCUSS_PHASE = path.join(
|
||||||
|
__dirname, '..', 'get-shit-done', 'workflows', 'discuss-phase.md',
|
||||||
|
);
|
||||||
|
|
||||||
|
describe('discuss-phase context fixes (#2549, #2550, #2552)', () => {
|
||||||
|
let src;
|
||||||
|
test('discuss-phase.md source exists', () => {
|
||||||
|
assert.ok(fs.existsSync(DISCUSS_PHASE), 'discuss-phase.md must exist');
|
||||||
|
src = fs.readFileSync(DISCUSS_PHASE, 'utf-8');
|
||||||
|
});
|
||||||
|
|
||||||
|
// ─── #2549: load_prior_context cap ──────────────────────────────────────
|
||||||
|
test('#2549: load_prior_context must NOT instruct reading ALL prior CONTEXT.md files', () => {
|
||||||
|
if (!src) src = fs.readFileSync(DISCUSS_PHASE, 'utf-8');
|
||||||
|
assert.ok(
|
||||||
|
!src.includes('For each CONTEXT.md where phase number < current phase'),
|
||||||
|
'load_prior_context must not unboundedly read all prior CONTEXT.md files',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('#2549: load_prior_context must reference a bounded read (3 phases or DECISIONS-INDEX)', () => {
|
||||||
|
if (!src) src = fs.readFileSync(DISCUSS_PHASE, 'utf-8');
|
||||||
|
const hasBound = src.includes('3') && src.includes('prior CONTEXT.md');
|
||||||
|
const hasIndex = src.includes('DECISIONS-INDEX.md');
|
||||||
|
assert.ok(
|
||||||
|
hasBound || hasIndex,
|
||||||
|
'load_prior_context must reference a bounded read (e.g., most recent 3 phases) or DECISIONS-INDEX.md',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
// ─── #2550: scout_codebase phase-type selection ──────────────────────────
|
||||||
|
test('#2550: scout_codebase must not instruct reading all 7 codebase maps', () => {
|
||||||
|
if (!src) src = fs.readFileSync(DISCUSS_PHASE, 'utf-8');
|
||||||
|
assert.ok(
|
||||||
|
!src.includes('Read the most relevant ones (CONVENTIONS.md, STRUCTURE.md, STACK.md based on phase type)'),
|
||||||
|
'scout_codebase must not use the old vague "most relevant" instruction without a selection table',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('#2550: scout_codebase must include a phase-type-to-maps selection table', () => {
|
||||||
|
if (!src) src = fs.readFileSync(DISCUSS_PHASE, 'utf-8');
|
||||||
|
// The table maps phase types to specific map selections
|
||||||
|
assert.ok(
|
||||||
|
src.includes('Phase type') && src.includes('Read these maps'),
|
||||||
|
'scout_codebase must include a phase-type to map-selection table',
|
||||||
|
);
|
||||||
|
// Key phase types must be covered
|
||||||
|
assert.ok(src.includes('UI') || src.includes('frontend'), 'Table must cover UI/frontend phases');
|
||||||
|
assert.ok(src.includes('Backend') || src.includes('API'), 'Table must cover backend phases');
|
||||||
|
assert.ok(src.includes('Testing'), 'Table must cover testing phases');
|
||||||
|
assert.ok(src.includes('Mixed'), 'Table must have a fallback for mixed/unclear phases');
|
||||||
|
});
|
||||||
|
|
||||||
|
// ─── #2552: no split reads ───────────────────────────────────────────────
|
||||||
|
test('#2552: scout_codebase must explicitly prohibit split reads of the same file', () => {
|
||||||
|
if (!src) src = fs.readFileSync(DISCUSS_PHASE, 'utf-8');
|
||||||
|
const prohibitsSplit = src.includes('split reads') || src.includes('split read');
|
||||||
|
assert.ok(
|
||||||
|
prohibitsSplit,
|
||||||
|
'scout_codebase must explicitly warn against split reads (same file, two offsets) that break prompt cache',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user