From 2acb38c9187a087e86d05912742345389a0d6061 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 16 Apr 2026 17:15:29 -0400 Subject: [PATCH] fix(pattern-mapper): prevent redundant file reads and add early-stop rule (#2312) (#2327) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add /gsd-spec-phase — Socratic spec refinement with ambiguity scoring (#2213) Introduces `/gsd-spec-phase ` as an optional pre-step before discuss-phase. Clarifies WHAT a phase delivers (requirements, boundaries, acceptance criteria) with quantitative ambiguity scoring before discuss-phase handles HOW to implement. - `commands/gsd/spec-phase.md` — slash command routing to workflow - `get-shit-done/workflows/spec-phase.md` — full Socratic interview loop (up to 6 rounds, 5 rotating perspectives: Researcher, Simplifier, Boundary Keeper, Failure Analyst, Seed Closer) with weighted 4-dimension ambiguity gate (≤ 0.20 to write SPEC.md) - `get-shit-done/templates/spec.md` — SPEC.md template with falsifiable requirements (Current/Target/Acceptance per requirement), Boundaries, Acceptance Criteria, Ambiguity Report, and Interview Log; includes two full worked examples - `get-shit-done/workflows/discuss-phase.md` — new `check_spec` step detects `{padded_phase}-SPEC.md` at startup; displays "Found SPEC.md — N requirements locked. Focusing on implementation decisions."; `analyze_phase` respects `spec_loaded` flag to skip "what/why" gray areas; `write_context` emits `` section with boundary summary and canonical ref to SPEC.md - `docs/ARCHITECTURE.md` — update command/workflow counts (74→75, 71→72) Closes #2213 Co-Authored-By: Claude Sonnet 4.6 * fix(pattern-mapper): prevent redundant file reads and add early-stop rule (#2312) Adds three explicit constraints to the agent prompt: 1. Read each analog file EXACTLY ONCE (no re-reads from context) 2. For files > 2,000 lines, use Grep + Read with offset/limit instead of full load 3. Stop analog search after 3–5 strong matches Also adds block to surface these constraints at high salience. Adds regression tests READS-01, READS-02, READS-03. Closes #2312 Co-Authored-By: Claude Sonnet 4.6 * fix(pattern-mapper): clarify re-read rule allows non-overlapping targeted reads (CR feedback) "Read each file EXACTLY ONCE" conflicted with the large-file targeted-read strategy. Rewrites both the Step 4 guidance and the block to make the rule precise: re-reading the same range is forbidden; multiple non-overlapping targeted reads for large files are permitted. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- agents/gsd-pattern-mapper.md | 16 ++++++++++++++++ tests/pattern-mapper.test.cjs | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/agents/gsd-pattern-mapper.md b/agents/gsd-pattern-mapper.md index a007513dc..fb850b29b 100644 --- a/agents/gsd-pattern-mapper.md +++ b/agents/gsd-pattern-mapper.md @@ -118,6 +118,12 @@ Grep("router\.(get|post|put|delete)", type: "ts") ## Step 4: Extract Patterns from Analogs +**Never re-read the same range.** For small files (≤ 2,000 lines), one `Read` call is enough — extract everything in that pass. For large files, multiple non-overlapping targeted reads are fine; what is forbidden is re-reading a range already in context. + +**Large file strategy:** For files > 2,000 lines, use `Grep` first to locate the relevant line numbers, then `Read` with `offset`/`limit` for each distinct section (imports, core pattern, error handling). Use non-overlapping ranges. Do not load the whole file. + +**Early stopping:** Stop analog search once you have 3–5 strong matches. There is no benefit to finding a 10th analog. + For each analog file, Read it and extract: | Pattern Category | What to Extract | @@ -297,6 +303,16 @@ Pattern mapping complete. Planner can now reference analog patterns in PLAN.md f + + +- **No re-reads:** Never re-read a range already in context. Small files: one Read call, extract everything. Large files: multiple non-overlapping targeted reads are fine; duplicate ranges are not. +- **Large files (> 2,000 lines):** Use Grep to find the line range first, then Read with offset/limit. Never load the whole file when a targeted section suffices. +- **Stop at 3–5 analogs:** Once you have enough strong matches, write PATTERNS.md. Broader search produces diminishing returns and wastes tokens. +- **No source edits:** PATTERNS.md is the only file you write. All other file access is read-only. +- **No heredoc writes:** Always use the Write tool, never `Bash(cat << 'EOF')`. + + + Pattern mapping is complete when: diff --git a/tests/pattern-mapper.test.cjs b/tests/pattern-mapper.test.cjs index 11116119f..435e0337a 100644 --- a/tests/pattern-mapper.test.cjs +++ b/tests/pattern-mapper.test.cjs @@ -1,11 +1,12 @@ /** - * Tests for Pattern Mapper feature (#1861) + * Tests for Pattern Mapper feature (#1861, #2312) * * Covers: * - Config key workflow.pattern_mapper in VALID_CONFIG_KEYS * - Default value is true * - Config round-trip (set/get) * - init plan-phase output includes patterns_path (null when missing, path when present) + * - Agent prompt contains no-re-read and early-stop constraints (#2312) */ const { describe, test, beforeEach, afterEach } = require('node:test'); @@ -109,3 +110,33 @@ describe('init plan-phase patterns_path', () => { assert.ok(data.patterns_path.includes('01-foundation'), `Expected path to include phase dir, got: ${data.patterns_path}`); }); }); + +describe('gsd-pattern-mapper agent prompt efficiency constraints (#2312)', () => { + const agentPath = path.join(__dirname, '..', 'agents', 'gsd-pattern-mapper.md'); + let agentContent; + + beforeEach(() => { + agentContent = fs.readFileSync(agentPath, 'utf-8'); + }); + + test('READS-01: prompt contains no-re-read constraint', () => { + assert.ok( + /read each.*file.*once/i.test(agentContent) || /never re-read/i.test(agentContent), + 'Agent prompt must instruct the model to read each analog file only once' + ); + }); + + test('READS-02: prompt contains early-stop instruction', () => { + assert.ok( + /stop.*analog|3.?5.*analog|early.stop/i.test(agentContent), + 'Agent prompt must instruct the model to stop after finding 3-5 analogs' + ); + }); + + test('READS-03: prompt contains large-file strategy', () => { + assert.ok( + /2[,.]?000.*line|offset.*limit|large file/i.test(agentContent), + 'Agent prompt must include guidance for reading large files with offset/limit' + ); + }); +});