doc: port harness-engineering practices into dev docs - #1014
Conversation
Adopt the practices from Ryan Lopopolo's harness-engineering corpus that fit this repo, so lessons from agent sessions compound instead of accreting as prose: - docs/dev/harness.md: how the harness (routed context, checks, skills, review loops) changes and proves its worth -- the improvement loop, fresh-session testing, the promotion ladder from steering to executable checks, fix-the-class sibling sweeps, carrying-cost ablation, and session learnings as telemetry needing corroboration. - docs/dev/design.md: product design context (users, brand, principles, tokens) moved out of the root CLAUDE.md, which stays a routing map; the root and the frontend packages link to it, and the i-teach-impeccable skill now writes there instead of recreating the section at the root. - Root CLAUDE.md PR guidance: PR bodies carry claim-matched evidence (which tests/journeys establish the behavior, the artifact carrying the claim, and what the evidence does not establish). - address-feedback skill: every review finding gets an explicit fix / defer-with-rationale / disagree-with-rationale response; disagreement is recorded in the cycle summary, never silently dropped. - docs/dev/rust.md: never pipe cargo test through head/tail -- head can SIGPIPE-kill the run, tail discards the evidence a late failure needs, and both can hide the exit status. - docs/dev/workflow.md: one-concept-one-owner warning signs and the current-state-is-data / durable-policy-is-code distinction. - src/simlin-mcp-core/CLAUDE.md: tool results are context for a consuming agent's next decision -- quiet success, bounded results, errors naming the violated invariant and repair. Also drops the stale 'Last reviewed' comment the root CLAUDE.md forbids.
Corrections from the review pass, each verified against the code: - simlin-mcp-core/CLAUDE.md claimed output types strip engine-internal fields; curation is input-side only -- both tool outputs embed the full engine json::Model, which serializes uid/compat when populated. Both the Files entry and the Tool Design bullet now state this, and the bullet warns that a new output payload does not inherit the input curation for free. - rust.md attributed head-truncation to a SIGPIPE kill; Rust tooling ignores SIGPIPE, so the producer sees EPIPE and can stop early while still exiting 0 (verified: cargo tree | head -2 drops ~178KB with PIPESTATUS 0 0). The rewritten mechanism matches the checked behavior. - The i-teach-impeccable skill's Step 3 template emitted a '## Design Context' block that mismatched docs/dev/design.md's structure; both the .claude and .agents copies now update the file's existing sections in place, and the .agents copy no longer targets a nonexistent .github/copilot-instructions.md. harness.md names .agents/skills as part of the sweep population, and the address-feedback-local variant there received the same fix/defer/disagree wording as the .claude skill. - docs/dev/design.md scopes its token table to the surfaces that load src/diagram/theme.css and notes the website's separate brand palette (--rp-c-brand: #c83639); website and simlin-serve CLAUDE.md gained the design.md routing line the other frontend packages already had. - The remaining 'Last reviewed' stamps in simlin-mcp and simlin-serve CLAUDE.md are gone, and check-docs.py now rejects such stamps in any CLAUDE.md (matched narrowly so the root CLAUDE.md prose describing the ban does not trip it). The walker also skips .claude/worktrees, which holds checkouts of other branches.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac39f5ec29
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| r'|^\s*Last\s+(?:reviewed|updated|verified)\s*:)', | ||
| re.IGNORECASE | re.MULTILINE, | ||
| ) | ||
| for match in stamp_re.finditer(raw_content): |
There was a problem hiding this comment.
Strip fenced blocks before scanning for stamps
When a CLAUDE.md file contains a fenced example with text such as Last updated: 2026-01-01, this scan runs against raw_content and rejects the example as a real stamp. That contradicts this checker's documented exclusion of fenced code blocks and can unnecessarily block the pre-commit hook; run the stamp regex against the already stripped content while retaining raw_content only for line-number calculation.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1014 +/- ##
==========================================
- Coverage 92.18% 92.17% -0.01%
==========================================
Files 247 247
Lines 158229 158229
==========================================
- Hits 145858 145855 -3
- Misses 12371 12374 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ReviewDocs-and-script change; I focused on the executable piece ( No blocking findings. Verified:
Minor observation (not a finding, just for your awareness): the stamp regex runs against Overall correctness: correct. |
The stamp check ran against raw_content, so a fenced example such as '<!-- Last updated: ... -->' in a doc would be rejected as a real stamp, contradicting the checker's documented exclusion of fenced code blocks. Scan content_links instead (fenced blocks stripped, inline code spans blanked), which also spares a backticked mention of the banned form; both transforms preserve line numbers, so reported locations stay accurate. Verified: the pre-change stamps still trip the check, and fenced/inline examples plus the root CLAUDE.md prose do not.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9bb86aca9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if file_path.name == "CLAUDE.md": | ||
| stamp_re = re.compile( | ||
| r'(<!--\s*Last\s+(?:reviewed|updated|verified)' | ||
| r'|^\s*Last\s+(?:reviewed|updated|verified)\s*:)', |
There was a problem hiding this comment.
Recognize Markdown-formatted stamp lines
Expand the start-of-line alternative to account for common Markdown decoration. A CLAUDE.md containing **Last updated:** 2026-08-10, ## Last verified: 2026-08-10, or - Last reviewed: 2026-08-10 currently passes this check because the regex requires Last to follow only whitespace. These remain genuine freshness stamps and therefore bypass the executable ratchet for the repository's explicit ban.
AGENTS.md reference: AGENTS.md:L122-L122
Useful? React with 👍 / 👎.
| ``` | ||
|
|
||
| Write this section to .github/copilot-instructions.md in the project root. If the file exists, append or update the Design Context section. | ||
| Synthesize your findings and the user's answers, then update `docs/dev/design.md` -- this repo's canonical home for product design context, routed from the root CLAUDE.md's Development Standards list. Update the relevant existing sections in place (`## Users`, `## Brand Personality`, `## Aesthetic Direction`, `## Design Principles`), preserving the file's `# Product Design Context` structure and leaving sections you did not gather new answers for (such as `## Design Tokens Reference` and `## Accessibility`) intact. Do not append a duplicate section or write design context anywhere else. |
There was a problem hiding this comment.
Align the skill description with its new output
Update the frontmatter description in both copies of this skill when changing the destination here. The catalog-visible description still promises to save the result to an “AI config file,” but this step now explicitly writes docs/dev/design.md and forbids writing it anywhere else. Since that description is what users and agents see while selecting the skill, it now advertises behavior the skill no longer performs.
AGENTS.md reference: AGENTS.md:L119-L119
Useful? React with 👍 / 👎.
|
Reviewed the docs-only changes plus the Verified:
Overall correctness verdict: correct. Docs-only content with one narrowly-scoped script ratchet; the ratchet was designed carefully enough to not false-positive on the prose describing the ban it enforces. |
Ports the practices from Ryan Lopopolo's harness-engineering corpus (
third_party/harness-engineering, a local untracked clone) that fit this repo, so lessons from agent sessions compound as durable structure instead of accreting as prose.What changed
docs/dev/harness.md(new) -- owns how the harness itself changes: the improvement loop (baseline trajectory, earliest failed handoff, smallest intervention at the owning boundary, fresh-session rerun, retain/revise/remove), fresh-session testing of new guidance ("the presence of a document proves only that it was written"), the promotion ladder from steering to executable checks, fix-the-class sibling sweeps with ratchets, carrying-cost ablation, and session learnings as telemetry that needs corroboration before becoming policy.docs/dev/design.md(new) -- the product design context (users, brand, principles, tokens, accessibility) moved out of the root CLAUDE.md, which stays a routing map. All four frontend surfaces' CLAUDE.md files route to it; the token table is scoped to surfaces that loadsrc/diagram/theme.css, with the website's separate red brand palette noted. Both copies of thei-teach-impeccableskill now update this file in place instead of recreating the section at the root (or in a nonexistent.github/copilot-instructions.md)..claudeand.agentstrees) -- every review finding gets an explicit fix / defer-with-rationale / disagree-with-rationale response; disagreement is recorded in the cycle summary, never silently dropped.docs/dev/rust.md-- never pipecargo testthroughhead/tail: Rust tooling ignores SIGPIPE, soheadtruncation exits 0 and a cut-short run is indistinguishable from a clean one (verified empirically in-repo).docs/dev/workflow.md-- one-concept-one-owner warning signs and the current-state-is-data / durable-policy-is-code distinction.src/simlin-mcp-core/CLAUDE.md-- tool results are context for a consuming agent's next decision: quiet success, bounded results, errors naming the violated invariant and repair. States accurately that field curation (uid/compat/aiState) is input-side only.scripts/check-docs.py-- a ratchet rejectingLast reviewed/updated/verifiedstamps in CLAUDE.md files (the two surviving stamps are removed here), and the walker now skips.claude/worktreescheckouts. This is the new harness.md ladder in action: the prose ban already existed; the invariant is now executable.Evidence
Docs-only change plus one script: both commits passed the full pre-commit suite (Rust fmt/clippy/test, TS lint/build/tsc/test, wasm build, pysimlin tests, dep policy, doc links, project lints). The check-docs.py ratchet was verified to fire on the pre-change stamped files and to not false-positive on the root CLAUDE.md prose that describes the ban. What this does not establish: whether the new guidance actually changes future agent behavior -- per harness.md's own standard, that takes fresh-session use, and the docs should be revised or removed if they don't earn their keep.
A
/code-reviewpass ran before this PR; all seven findings (output-type curation claim, SIGPIPE mechanism, skill template mismatch, unswept.agentssibling tree, missing website/serve routing lines, token-scope overreach, surviving stamps) were verified against the code and fixed in the second commit.🤖 Generated with Claude Code
https://claude.ai/code/session_012cWNaWAXQGcjM12RpVS2oa