feat(nap): measure charter/skill context and add --dry-run --json - #1987
Closed
bradygaster wants to merge 1 commit into
Closed
feat(nap): measure charter/skill context and add --dry-run --json#1987bradygaster wants to merge 1 commit into
--dry-run --json#1987bradygaster wants to merge 1 commit into
Conversation
Squad's `reskill` skill asked agents to hand-write a savings table, but no tool produced those numbers — every reskill figure was an estimate. This makes `squad nap --dry-run --json` the measurement primitive reskill was missing, rather than building a second engine. - NapMetrics gains charterBytes, skillBytes, charterReducibleBytes and historyReducibleBytes. Reducible = sum(max(0, size - TARGET)) against the targets already documented in reskill/SKILL.md (1536 charter, 8192 history). - NapResult gains `dryRun`; the report gets a dry-run banner and conditional verbs applied at format time, so a preview can no longer be misread as a completed run. - New `--json` flag on both the CLI and the REPL `/nap` path, following the existing health.ts JSON precedent. - reskill/SKILL.md now starts its audit from measured output and forbids invented savings figures. Mirrored to the SDK template copy. Nap measures charters and skills but never modifies them — automated charter rewriting stays an explicit non-goal, and estimateAfterMetrics passes those fields through unchanged rather than fabricating deltas. The added code introduces zero write operations. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
🟠 Impact Analysis — PR #1987Risk tier: 🟠 HIGH 📊 Summary
🎯 Risk Factors
📦 Modules Affecteddocs (2 files)
root (1 file)
squad-cli (5 files)
squad-sdk (1 file)
tests (2 files)
This report is generated automatically for every PR. See #733 for details. |
Contributor
🏗️ Architectural Review
Automated architectural review — informational only. |
Contributor
🛫 PR Readiness Check
PR Scope: 📦🔧 Mixed (product + infrastructure)
|
| Status | Check | Details |
|---|---|---|
| ✅ | Single commit | 1 commit — clean history |
| ✅ | Not in draft | Ready for review |
| ✅ | Branch up to date | Up to date with dev |
| ❌ | Copilot review | No Copilot review yet — it may still be processing |
| ✅ | Changeset present | Changeset file found |
| ✅ | Scope clean | No .squad/ or docs/proposals/ files |
| ✅ | No merge conflicts | No merge conflicts |
| ❌ | Copilot threads resolved | 4 unresolved Copilot thread(s) — fix and resolve before merging |
| ❌ | CI passing | 1 check(s) failing: test |
Files Changed (11 files, +1123 −30)
| File | +/− |
|---|---|
.changeset/nap-context-json-safety.md |
+5 −0 |
docs/src/content/docs/features/context-hygiene.md |
+15 −7 |
docs/src/content/docs/reference/cli.md |
+1 −0 |
packages/squad-cli/src/cli-entry.ts |
+11 −3 |
packages/squad-cli/src/cli/core/command-help.ts |
+3 −2 |
packages/squad-cli/src/cli/core/nap.ts |
+164 −10 |
packages/squad-cli/src/cli/shell/commands.ts |
+4 −0 |
packages/squad-cli/templates/skills/reskill/SKILL.md |
+15 −4 |
packages/squad-sdk/templates/skills/reskill/SKILL.md |
+15 −4 |
test/nap-reskill-measurement.test.ts |
+881 −0 |
test/nap.test.ts |
+9 −0 |
Total: +1123 −30
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
formatNapReport can misreport “saved” in real runs when totalBytes grows, and the new test helper doesn’t actually create an empty .squad/ directory.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 3
New issues introduced by this change (4)
| Severity | Finding |
|---|---|
 |
packages/squad-cli/src/cli/core/nap.ts — tokensSaved clamps negative deltas to 0 via Math.max(0, saved), but after.totalBytes can be… |
|
packages/squad-cli/src/cli/core/nap.ts — The overall summary line always says “saved/would save”, but saved can be negative in real runs… |
|
test/nap-reskill-measurement.test.ts — createTestSquadDir claims to create an isolated .squad/ directory, but when structure is… |
 |
test/nap-reskill-measurement.test.ts — PR description safety guarantee #1 says a diff grep for write/remove operations over added lines… |
What changed in this PR
This PR extends the existing squad nap engine to measure the charter/skill context that “reskill” is intended to reduce, and adds a --dry-run --json output mode so reskill-style savings can be based on deterministic, tool-produced numbers rather than estimates.
Changes:
- Add new
NapMetricsfields forcharterBytes,skillBytes,charterReducibleBytes, andhistoryReducibleBytes, plus a top-leveldryRunflag onNapResult. - Add
--jsonoutput forsquad napin both CLI and interactive shell, and improve dry-run report labeling/wording. - Add an evidentiary test suite for exact byte accounting + update reskill skill/docs to reference measured
nap --dry-run --jsonvalues.
| File | Description |
|---|---|
| test/nap.test.ts | Updates report-formatting fixture to include the expanded metrics shape and dryRun. |
| test/nap-reskill-measurement.test.ts | New byte-exact fixture-based tests for charter/skill/reducible metrics, dry-run safety, and JSON shape. |
| packages/squad-sdk/templates/skills/reskill/SKILL.md | Switches reskill guidance from estimates to measured nap --dry-run --json baselines and savings. |
| packages/squad-cli/templates/skills/reskill/SKILL.md | Mirrors the same reskill prompt update in the CLI templates. |
| packages/squad-cli/src/cli/shell/commands.ts | Adds --json support for /nap in the interactive shell (sync path). |
| packages/squad-cli/src/cli/core/nap.ts | Implements new measurements, adds dryRun to result, dry-run banner/conditional verbs, and reducibility reporting. |
| packages/squad-cli/src/cli/core/command-help.ts | Updates nap help text to document --json and the dry-run labeling behavior. |
| packages/squad-cli/src/cli-entry.ts | Adds --json support to the primary CLI nap command output. |
| docs/src/content/docs/reference/cli.md | Documents squad nap --json in the CLI reference table. |
| docs/src/content/docs/features/context-hygiene.md | Expands context-hygiene docs to recommend --dry-run --json for measured reclaimable context and reskill audit. |
| .changeset/nap-context-json-safety.md | Patch changeset describing the new metrics, JSON output, and dry-run UX changes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const { before, after, actions } = result; | ||
| const { before, after, actions, dryRun } = result; | ||
| const saved = before.totalBytes - after.totalBytes; | ||
| const tokensSaved = humanTokens(Math.max(0, saved)); |
Comment on lines
+766
to
768
| const arrow = dryRun ? `${D}(would become)${R}` : `${D}->${R}`; | ||
| lines.push(` ${humanBytes(before.totalBytes)} ${arrow} ${G}${humanBytes(after.totalBytes)}${R} ${D}(${dryRun ? 'would save' : 'saved'} ~${tokensSaved} tokens)${R}`); | ||
| lines.push(''); |
Comment on lines
+66
to
+70
| const tmpDir = mkdtempSync(join(tmpdir(), 'squad-nap-reskill-')); | ||
| tmpDirs.push(tmpDir); | ||
| const squadDir = join(tmpDir, '.squad'); | ||
| for (const [filePath, content] of Object.entries(structure)) { | ||
| const fullPath = join(squadDir, filePath); |
Comment on lines
+31
to
+37
| import { | ||
| mkdtempSync, | ||
| writeFileSync, | ||
| readFileSync, | ||
| readdirSync, | ||
| mkdirSync, | ||
| rmSync, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
squad napis a real 730-line engine with metrics, dry-run, and tests.reskillis a 92-line prompt with no code, no measurement, and no test.reskill/SKILL.mdasked agents to fill in a savings table — but no tool in the repository produced those numbers. Every reskill savings figure ever reported was a hand-estimate.Separately,
nap'sNapMetricsmeasured history/log/decision bytes but not charter or skill bytes — the two things reskill exists to shrink. So the tool that could measure didn't measure the right thing, and the tool that needed numbers had no tool.Flight's ruling: don't build a second engine. Extend nap's measurement so
squad nap --dry-run --jsonbecomes the primitive reskill was missing. Ship the primitive; defer the engine.Baseline vs improved measurement
Everything below is asserted with exact equality in
test/nap-reskill-measurement.test.tsagainstmkdtempfixtures.charterBytes9036charterReducibleBytes3928skillBytes3072historyBytes5100051000historyReducibleBytes21424Reskill opportunity surfaced: 25,352 bytes (24.8 KB) per spawn — previously unmeasurable.
reducible = Σ max(0, size − TARGET), whereCHARTER_TARGET = 1536andHISTORY_TARGET = 8192come from the targets already written inreskill/SKILL.md(lines 31 and 39) and are commented with that provenance, so they're traceable rather than magic.Real-world datapoint
Run against this repo's own live
.squad/(220 files, 1.7 MB):→ ~30 KB of reskill headroom that no tool could previously report.
Safety guarantees
git diff | grep -E 'writeFileSync|rmSync|unlinkSync|renameSync|mkdirSync|truncate'over the added lines returns nothing. All new code is measurement and formatting.after.charterBytes === before.charterBytesandafter.skillBytes === before.skillBytesafter a real (non-dry-run) nap.estimateAfterMetricspasses charter/skill fields through unchanged rather than fabricating deltas for work nap doesn't do — a fabricated delta would be a lie the caller couldn't distinguish from truth.nap --dry-run --jsonran against the real 220-file.squad/above and modified zero files (confirmed byfind -newermt).isCommittableDestination(Scribe's decisions archive step deletes entries without appending them to decisions-archive.md #1774 / Archival can silently destroy state: writes to untracked destinations are deletions #1783) were declared off-limits and are unchanged.--jsonis opt-in; the dry-run banner is presentation-only.formatNapReport(result, noColor?)keeps its signature —dryRunis read offresult.Limitations (stated honestly)
historyReducibleBytespasses through in dry-run. The aggregate can't be exactly decomposed from per-filehistoryBytesdeltas, so dry-run reports the pre-nap value. Honest over clever.--jsonstay past-tense. Conditional verbs (Compressed→Would compress) are applied at format time viadryRunifyDescription(). JSON consumers key onresult.dryRun, not on prose. Deliberate — prose in a machine payload shouldn't be the source of truth.before.totalBytes − after.totalBytes === Σ bytesSavedholds exactly in dry-run only. In a real run,compress/archive/mergemove content into sibling files still under.squad/; onlypruneis a pure deletion. The real-run test assertssavings > 0with the reason inline.squad reskillengine. Deliberately deferred to its own PR now that the measurement primitive exists.Tests
test/nap-reskill-measurement.test.ts— 24 new tests in 7 groups (881 lines), including boundary math at 1535/1536/1537 and 8191/8192/8193, non-.mdfiltering in skills traversal, and dry-run vs real-run pass-through semantics.All fixtures use
mkdtempSync(join(tmpdir(), ...))— no test touches a real.squad/.Team
nap.ts+ all call sites (CLI, REPL, help)reskill/SKILL.mdfrom estimate-based to measurement-based; verified SDK mirror paritycontext-hygiene.md,cli.mdNapMetricsis not publicly exported →patchNotes for reviewers
runNapandrunNapSyncare near-duplicate implementations; every change landed in both or the CLI and REPL (/nap,/compact) would silently drift.packages/squad-sdk/templates/skills/reskill/SKILL.mdmirrors the CLI copy viascripts/sync-skill-templates.mjs(runs onprebuild). Parity verified withdiff— identical.-build.1, CRLF↔LF on 9workflow-wiring-*templates) was reverted so the diff stays scoped to the 11 intended files.Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com