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>
This commit is contained in:
@@ -39,6 +39,18 @@ Unix sockets. You are implementing it one task at a time.
|
|||||||
- Keep struct fields and enum variants in the order the task gives. The order is the wire format.
|
- Keep struct fields and enum variants in the order the task gives. The order is the wire format.
|
||||||
- Comments say why, not what. Match the amount of commenting you see in the task's examples.
|
- Comments say why, not what. Match the amount of commenting you see in the task's examples.
|
||||||
|
|
||||||
|
## Lessons from earlier reviews
|
||||||
|
|
||||||
|
These come from defects found in review. The evidence is in `docs/implementer-lessons.md`.
|
||||||
|
|
||||||
|
- 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 a task shows one place; the rule covers all of them.
|
||||||
|
- A check must fail when it cannot do its job: missing input, unreadable file, a tool that errors.
|
||||||
|
Never throw errors away with `2>/dev/null`, `|| true` or an ignored `Result`.
|
||||||
|
- Report every problem you find, not only the first.
|
||||||
|
- Log every attempt, including one you abandon.
|
||||||
|
|
||||||
## The gate
|
## The gate
|
||||||
|
|
||||||
`make gate` must print `gate: ok` before a task is done. It runs offline: rustfmt, clippy with
|
`make gate` must print `gate: ok` before a task is done. It runs offline: rustfmt, clippy with
|
||||||
|
|||||||
@@ -4,9 +4,9 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
|
|||||||
|
|
||||||
## Current state
|
## Current state
|
||||||
|
|
||||||
Boxmaker is a sovereign personal agent harness written in Rust. There is no Rust code yet. Work
|
Boxmaker is a sovereign personal agent harness written in Rust. Work proceeds one milestone at a
|
||||||
proceeds one milestone per session (M0 to M7, table in `docs/milestones.md`). M0 is done; check
|
time (M0 to M7, table in `docs/milestones.md`). M0 (measurements) and M1 (workspace, `proto`, gate,
|
||||||
for `AGENTS.md` and `Cargo.toml` to see whether M1 has started.
|
`Decision`) are done. Check `docs/plans/` and `docs/implementer-log.md` for what is in flight.
|
||||||
|
|
||||||
- `docs/design.md` is the binding design brief. If it looks wrong or conflicts with a measurement,
|
- `docs/design.md` is the binding design brief. If it looks wrong or conflicts with a measurement,
|
||||||
stop and say so. Changes to it land as their own commit and are recorded in `docs/decisions.md`,
|
stop and say so. Changes to it land as their own commit and are recorded in `docs/decisions.md`,
|
||||||
@@ -17,6 +17,9 @@ for `AGENTS.md` and `Cargo.toml` to see whether M1 has started.
|
|||||||
and `files/` under it holds the tests and fixtures the tasks copy in. `AGENTS.md` holds the
|
and `files/` under it holds the tests and fixtures the tasks copy in. `AGENTS.md` holds the
|
||||||
implementer's standing rules; its code rules apply to any code written here too.
|
implementer's standing rules; its code rules apply to any code written here too.
|
||||||
- `docs/implementer-log.md` is Laguna's own record, one row per task. Reviews go at the bottom.
|
- `docs/implementer-log.md` is Laguna's own record, one row per task. Reviews go at the bottom.
|
||||||
|
- `docs/implementer-lessons.md` turns review findings into tips, for Laguna and for writing its
|
||||||
|
tasks. Every milestone review updates it, and short general rules are promoted into `AGENTS.md`.
|
||||||
|
Do not edit the repo while Laguna has a task in flight: its tasks require a clean working tree.
|
||||||
- `spike/` is throwaway measurement code, not harness code.
|
- `spike/` is throwaway measurement code, not harness code.
|
||||||
|
|
||||||
Roles: implementation is done by Laguna S 2.1 (served by straylight) through OpenCode on this
|
Roles: implementation is done by Laguna S 2.1 (served by straylight) through OpenCode on this
|
||||||
@@ -27,14 +30,16 @@ closed tasks: exact paths, given type signatures, verified crate APIs, tests spe
|
|||||||
The inference server is shared with other sessions. Before using Ornith slot 1 or sending unpinned
|
The inference server is shared with other sessions. Before using Ornith slot 1 or sending unpinned
|
||||||
requests, check `GET /slots?model=ornith-1.5-35b-a3b` so you do not evict someone's cache.
|
requests, check `GET /slots?model=ornith-1.5-35b-a3b` so you do not evict someone's cache.
|
||||||
|
|
||||||
## Commands (planned in M1, not yet present)
|
## Commands
|
||||||
|
|
||||||
- `make gate` runs offline: `cargo fmt --check`, clippy with warnings denied, `cargo test`,
|
- `make gate` runs offline: `cargo fmt --check`, clippy with warnings denied, `cargo test`,
|
||||||
`cargo-deny`, and a check that fails on any source file over 500 lines. Run it before calling
|
`cargo-deny`, and a check that fails on any source file over 500 lines. Run it before calling
|
||||||
any work done, and report the exit status and last lines.
|
any work done, and report the exit status and last lines.
|
||||||
- `make verify-device` runs the checks that need straylight, such as the baseline token budget.
|
- `make verify-device` runs the checks that need straylight, such as the baseline token budget.
|
||||||
- Single test: `cargo test -p <crate> <test_name>`.
|
- Single test: `cargo test -p <crate> <test_name>`.
|
||||||
- `bxctl` is the owner CLI (`bxctl chat` from M2, `bxctl reindex` from M5).
|
- `make audit` runs `cargo deny check advisories`; it needs the network.
|
||||||
|
- `bxctl` is the owner CLI (`bxctl chat` from M2, `bxctl reindex` from M5). Until then every binary
|
||||||
|
prints "not implemented" and exits 2.
|
||||||
|
|
||||||
## Architecture in brief
|
## Architecture in brief
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,61 @@
|
|||||||
|
# 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.
|
||||||
@@ -59,3 +59,29 @@ by a later task, and all were found by reading the branch and probing it from ou
|
|||||||
easy case, though: types pinned by byte-exact fixtures. M2 has behaviour that fixtures cannot pin
|
easy case, though: types pinned by byte-exact fixtures. M2 has behaviour that fixtures cannot pin
|
||||||
as tightly (a streaming HTTP client, the turn loop), so an early mistake there is more likely to
|
as tightly (a streaming HTTP client, the turn loop), so an early mistake there is more likely to
|
||||||
be built on.
|
be built on.
|
||||||
|
|
||||||
|
### M1, tasks 08 and 09 — reviewed 2026-09-17 by the design model (Claude)
|
||||||
|
|
||||||
|
**Verdict: accepted. M1 is complete.** Both tasks passed the gate on the first run.
|
||||||
|
|
||||||
|
| Check | Result |
|
||||||
|
|---|---|
|
||||||
|
| Two commits with the trailer; only the listed paths staged; copied files identical to the plan; protected files untouched | pass |
|
||||||
|
| `make gate` | `gate: ok`, 50 tests |
|
||||||
|
| Reviewer's probes from the first review, run again | unknown fields rejected in `AuditRecord` and `ToolCall`; out-of-range timestamps are `Err`, no panic |
|
||||||
|
| No `2>/dev/null` left in the scripts; each fails when `crates/` is missing | pass |
|
||||||
|
|
||||||
|
Task 08 was the smallest correct change: one attribute on each struct, `Timestamp::MAX`, a fallible
|
||||||
|
`from_unix_millis`, `parse` routed through it, `now()` clamped.
|
||||||
|
|
||||||
|
Task 09 generalised beyond its self-test. The reviewer tried forms the self-test does not contain
|
||||||
|
and the scripts handled them: a `[target.'cfg(unix)'.dependencies]` section, a table-form
|
||||||
|
dependency under it, `[build-dependencies]`, a table header with spaces, a multi-line inline table,
|
||||||
|
and a commented-out dependency.
|
||||||
|
|
||||||
|
Remaining, recorded and not worth a task:
|
||||||
|
|
||||||
|
| # | Severity | Finding |
|
||||||
|
|---|---|---|
|
||||||
|
| 8 | low | `check-lines.sh` stops at the first file that is too long, so a second one is only reported after the first is fixed. |
|
||||||
|
| 9 | low | A quoted key (`"brokerd" = { path = "…" }`) is not seen by either dependency script. The task did not list that form and the reference scripts miss it too. It does not happen by accident. The robust fix is to ask `cargo metadata`, which needs a JSON parser the gate does not have. |
|
||||||
|
|||||||
Reference in New Issue
Block a user