Files
boxmaker/docs/implementer-lessons.md
T
kyleandClaude Fable 5.1 57bfc62236 Accept M1 follow-ups; add implementer lessons and promote four rules to AGENTS.md
Tasks 08 and 09 pass the checklist and the reviewer's probes, so M1 is
complete. docs/implementer-lessons.md turns the review findings into
tips for the implementer and for task writing, each tied to its
evidence, with a column to record whether the defect comes back.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-17 10:19:06 -07:00

5.5 KiB

Implementer lessons

What reviews of the implementing model's work (Laguna S 2.1 through OpenCode) have taught us, as tips. It has two audiences: the implementer, and whoever writes its tasks. Every tip comes from a finding in docs/implementer-log.md. A tip without evidence does not belong here.

How it is used:

  • After each milestone review, each finding becomes a row here or confirms an existing one.
  • Short, general rules are promoted into AGENTS.md, which the implementer reads in every session and which must stay small. The "In AGENTS.md" column says which.
  • "Seen again" is filled in at later reviews. A tip that does not stop its defect from coming back needs rewording or a test, not repetition.

Tips for the implementer

# Tip Evidence In AGENTS.md Seen again
I1 When a rule says "every" or "everywhere", finish by listing each place it could apply (every struct and enum in the file, every script, every branch) and check them one by one. An example in the task shows one place; the rule covers all of them. M1 finding 1: deny_unknown_fields was put on both enums in task 06, where the task's example showed it, and on neither struct. In task 03, where the task said "works on structs and on enums", every type got it. yes
I2 A check must fail when it cannot do its job: missing input, unreadable file, a tool that errors. Never throw errors away (2>/dev/null, || true, an ignored Result). M1 finding 4: all three gate scripts passed when crates/ did not exist, and hid find and awk errors. yes
I3 Report every problem found, not only the first. M1 finding 8: check-lines.sh exits at the first long file. yes
I4 Run the formatter before the gate, and let it decide ordering. M1 finding 7: three first-gate failures were rustfmt reordering lib.rs. yes
I5 Log every attempt, including one you abandon. M1 review: the task 01 row mentions files "from a prior attempt" that has no row of its own. yes
I6 Prefer a conversion that can fail (u32::try_from) to a cast that is only safe because of a check somewhere else (as u32). M1 finding 6: bounded as casts in frame.rs. Correct today, but the safety depends on a line ten lines away. no (already implied by the code rules)

Tips for writing tasks

# Tip Evidence
T1 The implementer follows examples more reliably than prose. For a rule that applies in several kinds of place, show an example of each kind, or say in so many words that it applies to structs and enums alike. Same evidence as I1. The one task whose note covered both kinds had no miss.
T2 A test for a property that must hold "everywhere" should walk the data, not sample it. M1 finding 1 got through because the given tests checked unknown fields in a few hand-picked places. tests/strict.rs now adds an unknown key to every object of every fixture.
T3 When a rule depends on a file format, list every way the format can write the same thing. M1 finding 3: the task said "however it is written" and named three spellings; TOML has a fourth, the [dependencies.name] table, and the dependency check passed it.
T4 If formatting or serialising a value can fail, make the value impossible to construct. Check what the library does at the edges of its range before writing the signature. M1 finding 2: Timestamp accepted any u64; humantime cannot format past year 9999 and to_string() panics. The reference implementation had the same bug, so the given tests could not catch it.
T5 A reference implementation proves the tests can pass. It does not prove they are complete: reference and tests share the author's blind spots. Probe the finished work from outside with inputs the tests do not contain. Findings 2 and 3 were in the reference too. Both were found by probing, not by the gate.
T6 Do not tell the implementer how to lay out what a formatter controls. M1 finding 7.
T7 A follow-up task works well as: what the reviewer observed, which part was the task's fault, a failing test to copy in, and the new rules. Tasks 08 and 09 each passed the gate on the first run, and 09 handled six forms its self-test did not contain.

What worked and should be kept

  • Byte-exact fixtures, compared in both directions. No wire-format defect reached review.
  • Tests and fixtures copied in task by task and never edited. All 23, then 25, files were byte-identical at both reviews.
  • A step that makes the implementer prove a test has teeth (task 07: make new public, watch the compile_fail doctest fail, change it back). It was done and logged.
  • "Stop and report" instead of improvising. There were no invented dependencies, no #[allow], no edited tests and no touched design documents across nine tasks.
  • One task, one fresh session, one commit, with only listed paths staged. The history reads as the plan.

What we have seen of this implementer so far

Nine tasks, all in M1, all small and pinned by tests, so treat these as first impressions.

  • Follows process rules exactly: branch, staging, trailer, log, protected files.
  • Writes idiomatic Rust without panics or unsafe shortcuts, and adds sensible things the task did not ask for (FrameError::source).
  • Misses the unshown half of a general rule (I1).
  • In shell, reaches for defensive habits that hide failures (I2).
  • Given a clear defect report and a failing test, fixes the defect minimally and generalises.
  • Gate on the first run: 6 of 9 tasks. Wall time per task, including the owner's turnaround: about 10 to 55 minutes.