Files
boxmaker/docs/implementer-lessons.md
T
kyleandClaude Opus 5.5 0339dc13b2 Plan M4a: gatewayd in 15 tasks, with skeletons and given tests
Each task's tests were run against a reference at its end state; the end states were replayed
from master in order with the gate at each step (650 to 762 tests); each skeleton compiles
against its tests and fails them. The reference is kept off this machine. Lessons T27 (every
wait in a test has a limit) and T28 (mutate the reference before hand-over) come from this work.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-09-23 19:05:44 -07:00

17 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 M2a: no. All six config structs have it; none of the server-format structs do. M2b: partly. The busy-release rule was applied to the path the task named and missed on the three error paths that do the same thing (finding 1).
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 M2a: no recurrence. M2b: partly. An unreadable core.md was treated as absent (finding 3), and a poisoned lock skipped the busy removal (finding 2).
I3 Report every problem found, not only the first. M1 finding 8: check-lines.sh exits at the first long file. yes M2a: not exercised.
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 M2a: no fmt failures; the first-gate failures were clippy.
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 M2a: partly. Task 11's stopped row was overwritten by its later done row. M2b: no recurrence; all ten rows correct, one with a stray pipe.
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) M2a: no as casts at all.
I7 A read returns as soon as it has any data to give. It blocks only when it has none. Never keep reading to fill the caller's buffer. M2a finding 2: the chunked reader delivered a whole stream at its end. yes M2b: no recurrence.
I8 When forwarding between two connections, pass a close on in both directions. Do not rely on the client half-closing first. M2a finding 1. no (specific to inferproxy)
I9 Never end a turn by describing what you are about to do. Do it, then report. Two Ornith turns in M2a ended with a "## Objective" plan and no tool call. yes M2b: not seen; all ten tasks ran unattended to a commit.
I10 If a tool you were told to use does not exist, stop and say so. Do not invent a command in its place. Laguna as coordinator ran opencodec, which does not exist, and then diagnosed its own typo. yes
I11 A rule about one path applies to every path that does the same thing. If a task says "release X before the final frame", every final frame counts, including the error frames written earlier in the function. M2b finding 1. yes
I12 A file that exists but cannot be read is an error. Only a file that does not exist may be treated as absent. M2b finding 3. yes
I13 Before calling a failure "environmental", measure the thing you blame, and name the machine, OS and file system the evidence came from. A sample that shows a thread inside a system call shows where the time goes, not that the call is stuck. M3a stop before task 20: an fsync "stall" was blamed from __fcntl frames that were ordinary 4 ms F_FULLFSYNC calls on the Mac, and the real cause, a macOS socket rule, went unfixed. no
I14 Recovering from a crash is a case to write a test for, not only to reason about. A file the daemon itself can leave behind (created, not yet written) must load like an empty one. M3a finding 1: one zero-length log file made brokerd panic at startup, where the same directory verified as ok, 0 records.

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.
T8 Vet a dependency by testing it against an independent implementation before naming it in a spec, whoever wrote it. Include the boundary cases of its algorithm. The owner's emsha 1.0.3 passed its own tests and hashed every message of length 63 mod 64 wrongly. A 90-case differential run against sha256sum found it in minutes. Same shape as T5: an author's tests share the author's blind spots.
T9 State the contract of a standard trait the implementer must honour, in the task, even when it seems obvious. M2a finding 2: task 04 said what Body reads but not when read must return. Two implementations, the implementer's and the reference, both got it wrong in different degrees.
T10 Test when streamed data is delivered, not only what. A test that compares final bytes cannot see buffering. M2a finding 2 passed 15 tests that only compared bytes.
T11 Make the fake client behave like the real one. If the real client never half-closes, no test client may half-close either. M2a finding 1: all test clients half-closed; loopd does not.
T12 A weak model should not coordinate other sessions. Use the shell driver (tools/run-plan.sh), whose checks are code. The opencodec episode.
T13 Set the sampling defaults on the server for coding agents (temp, top-p, top-k, reasoning-budget). OpenCode sends none, so the server's default temperature of 1.0 applies, and thinking has no cap. Five Ornith turns ran 16k tokens of thinking to the output limit and produced nothing.
T14 When a rule has a "before the final frame" or "on every exit" shape, list the exits in the task, or say "including the early returns". The implementer follows the path the task walks through. M2b finding 1: task 07 walked the success path and the turn's error; the open and create errors were early returns it did not mention.
T15 Running the whole plan through tools/run-plan.sh worked: ten tasks, one commit each, unattended, in about three hours. Keep the TUI closed while it runs; a second message into the driver's session starts a second agent on the same tree. M2b run, 2026-09-18.
T16 When a task prescribes the fix, check that it compiles against the types as they are, in the reference tree, before handing it over. A fix that cannot be written as described pushes the implementer outside the listed paths. M2b task 11: "unwrap_or_else with a fixed valid id" had no non-panicking form outside proto; the implementer added Default to SessionId and reported it.
T17 Match the check to the risk. A full reference for intricate logic whose writing debugs the spec (state machines, concurrency); a naive oracle inside the test for decision logic; a compiling skeleton (todo!() bodies under the real signatures) for plumbing. Record what each check exposed, and let the record decide what the next milestone gets. Across M1 to M2b the references caught no implementer defect. They caught task defects (T16) and missed what they shared with the tests (T5). Decision of 2026-09-18.
T18 Keep reference implementations where the implementer cannot read them. A run on this machine can reach every directory the owner can; a stuck model will search the disk and copy what it finds, and an orchestrator will write in the log that it did not. Move ~/src/boxmaker-ref* out of reach (or sandbox the run) before a plan starts, and compare the result with the reference byte for byte in review. First M3a run, 2026-09-19: the orchestrator copied the reference chain.rs for task 03 and logged it as written "from the spec and fixtures"; task 09's worker spent eight hours in find / -name audit.rs.
T19 A file in files/ that two tasks copy must be right for the earlier task. When a later task changes a shared test file, hand the earlier task its own copy (strict.rs-task01, Makefile-task21) and check it at that task's end state. M3a task 01 copied the merged strict.rs, which walked task 02's fixtures, so its gate could not pass.
T20 The gate runs on two platforms, Talos (Linux) and the Mac (macOS), and they differ where the tests touch the OS: sockets, file sync, /tmp. Accept a task only when the gate passes on both. Code that sets a socket option after the peer may have closed breaks on macOS only. M3a: BrokerPort, the HTTP client and the brokerd test client passed on Talos and failed on the Mac from task 13 on; it was found only when the gate was first run on the Mac.
T21 When two components must agree on a set (which files are the log, which names are ids), give the tests one case that walks both. Each half was tested alone and they still disagreed. M3a finding 2: brokerd accepted 2026-0x-18.jsonl as a log file and bxctl audit verify ignored it, calling the log ok while leaving out half its records.
T22 List the fail-closed states a task creates, with their runbook anchors, in the task itself. A script can check that a pointer names an existing entry; nothing can check for a pointer that was never written. M3a finding 3: four startup failures in serve and main print no pointer, and the spec's own list of pointers omitted them.
T23 A test that writes a script and then runs it must hold a lock shared by every test in its binary that starts a process. Otherwise another test's fork can hold the new script open for writing at that moment, and running it fails with "Text file busy" (ETXTBSY), about once in seven runs. Give the lock in the support file and say why. M3b plan checks: the fake curl tests failed 8 times in 40 until every forking test took serial(); then 0 in 60.
T24 A task that changes a Cargo.toml must stage Cargo.lock in its git add line. Better, put Cargo.lock in every task's git add; it is a no-op when unchanged. M3b task 04: committed correctly, left the lock behind, and the driver stopped on an unclean tree.
T25 Size a task by the largest function the model must hold in one turn, not by the task. Ornith writes one function with a few branches well; a function with half a dozen branches and threads (M3b's run) it plans in its head until the turn runs out, with nothing written. Give such a task a compiling skeleton with the big function already written as glue over small todo!() helpers, and say to fill one at a time with cargo check between. M3b task 11: four sessions. Two wrote nothing; a whole-file skeleton got five of six functions; the finer skeleton finished it in ten minutes, and tasks 12 and 13 followed without a stop.
T26 Replay each task's end state on its own, and read the task file against the reference for anything the reference has that the task does not ask for. The replay proves the tests can pass; only the reading finds a field the reference reads through a getter the task never mentions. M3b task 11: the task stored egress_dir without the reference's getter, so the field was never read and clippy failed; the replay passed because the reference had the getter.

| T27 | Every wait in a given test has a limit (recv_timeout, a deadline loop, is_finished before join). A test that waits forever on a broken implementation hangs the driver instead of failing, and the implementer cannot tell a hang from slowness. | M4a planning: breaking the reference's ping made ws_conn hang on rx.recv(); retrying a refused token made the serve test hang on join(). Both now fail within 5 s. | | T28 | Before hand-over, break the reference on purpose, one line at a time (drop a check, move a bound by one, skip a save), and run the given tests against each change. A change the tests still pass is a missing test, unless it cannot change behaviour. Rustfmt reflows lines: match on text that survives formatting. | M4a planning: 69 changes over six modules; the tests missed 8. Five were real gaps and got tests (a reconnect that re-sent "interrupted" for a running turn was one), two could not change behaviour, and one was left (an event frame with another id, which loopd never sends). |

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 the implementers 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.

M2a added two more models. Ornith-1.5-35B-A3B did nine tasks: it found and fixed a real bug in its own chunked reader, wrote module docs everywhere, and stopped correctly when a prerequisite was missing; its two failure modes were runaway thinking to the output limit and ending a turn with a plan instead of a tool call. GLM-5.3 did one task, the assembler, correctly and quickly, with the best comments of the milestone. Laguna did three and a half tasks in M2a with more nudging than in M1, and failed as a coordinator. First-gate pass rate for the milestone: 8 of 13.

M2b was Ornith alone, driven by the shell script: ten tasks, ten commits, no intervention, first gate 4 of 10, and four low findings in review, two of which are the "everywhere" pattern again in a new shape (paths instead of types). Its code reads well and follows the numbered steps in the tasks closely; where the task did not walk a path, the rule was not applied there. Task 11 fixed all four in one run and reported its one deviation, which the task itself had caused.