ASSESSMENT.md: lessons from 28 Ornith tasks (v0–v2.3) and sift P5a so far
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
+210
@@ -0,0 +1,210 @@
|
|||||||
|
# Working with Ornith: an assessment from crossbar (and sift P5a)
|
||||||
|
|
||||||
|
Written 2026-09-25 by Claude, the owner/reviewer for these runs, at Kyle's request. It covers
|
||||||
|
every Ornith task run in this repository (plans v0 through v2.3) plus the first task of sift's
|
||||||
|
P5a plan, which uses the same process. The evidence is `docs/implementer-log.md`, the "Changes
|
||||||
|
during the run" sections of each `docs/plans/*/README.md`, and the commits.
|
||||||
|
|
||||||
|
## 1. Setup
|
||||||
|
|
||||||
|
- **Model:** `ornith-1.5-35b-a3b` (MoE, about 3B active) on straylight's llama-server router
|
||||||
|
(llama.cpp b10964, 4 unified slots, 262k context). Driven by OpenCode (`opencode run --pure -m
|
||||||
|
llama.cpp/ornith-1.5-35b-a3b`) from a clean checkout on the same machine.
|
||||||
|
- **Process** (adapted from Kyle's boxmaker):
|
||||||
|
- The owner writes a plan: a README, one task file per task, and **given acceptance tests**
|
||||||
|
under `_files/`.
|
||||||
|
- There is no reference implementation. The given tests are checked against a panic-only
|
||||||
|
skeleton of every new name and must fail there for the intended reasons.
|
||||||
|
- `tools/run-plan.sh` runs each task in a **fresh session**. It stops on:
|
||||||
|
- no commit;
|
||||||
|
- a dirty tree;
|
||||||
|
- no `done` row in the implementer log;
|
||||||
|
- (sift) a missing attribution trailer.
|
||||||
|
- `AGENTS.md` holds the standing rules: protected files, never edit a given test, stop and
|
||||||
|
report on contradictions, the gate.
|
||||||
|
- **Review:** after each plan, the owner reviews it (gate, `-race -count=3`, smoke, byte-identical
|
||||||
|
given files, probes outside the tests) and merges `--no-ff` to master.
|
||||||
|
|
||||||
|
## 2. Results in numbers
|
||||||
|
|
||||||
|
| plan | tasks | first real gate passed | correct stops | owner faults found | model faults found | notes |
|
||||||
|
|---|---|---|---|---|---|---|
|
||||||
|
| v0 | 5 | 5/5 | 0 | 2 | 2 (unchecked `Flusher` assertion; a log row that under-reported deviations) | 6–10 min per task |
|
||||||
|
| v0.1 | 1 | 1/1 | 0 | 0 | 0 | |
|
||||||
|
| v1 | 8 | 6/8 sessions that reached the gate | 1 | ~12 | 4 (no accounting row on client cancel; `/usage` → `null`; edited protected files; one refusal-ending) | ~4 h wall; task 06 oversized and split |
|
||||||
|
| v1.1 | 1 | 1/1 (4th session) | 0 | 2 | 3 refusal-endings | the fix was correct from session 2 |
|
||||||
|
| v2 | 5 | 5/5 | 1 | 5 | 2 refusal-endings, 1 malformed tool call; a real v1 defect surfaced (served 200 recorded as 499) | |
|
||||||
|
| v2.1 | 2 | 2/2 | 0 | 2 | 1 refusal-ending | |
|
||||||
|
| v2.2 | 2 | 1/2 (01 failed its first gate after the restart) | 0 | 2 | 0 | 21 and 8 min |
|
||||||
|
| v2.3 | 4 | 4/4 | 1 | 2 | 1 timing hack, 1 refusal-ending, 1 malformed tool call, 1 duplicate call | |
|
||||||
|
| sift P5a 01 | 1 | (in progress) | 0 | 1 | 1 refusal-ending, 1 hung session (tooling) | 55 min lost to an owner test bug |
|
||||||
|
|
||||||
|
**Totals:** 28 crossbar tasks, all finished and merged.
|
||||||
|
- **Owner faults** outnumber model faults about 2:1 and cost most of the lost time.
|
||||||
|
- **Model faults** are overwhelmingly process faults (ending a turn early), not wrong code.
|
||||||
|
- **Defects in Ornith's code that reached review:** 6 over 28 tasks, all small or medium. One
|
||||||
|
(release-on-first-flush, v2.3) would have silently disabled the limiter for streams.
|
||||||
|
|
||||||
|
## 3. What Ornith does well
|
||||||
|
|
||||||
|
- **It writes correct Go to a clear spec.** Nearly every task that reached the gate passed it on
|
||||||
|
the first real run. The code is idiomatic, stays under the line limits once told where to put
|
||||||
|
things, and follows the error and logging rules.
|
||||||
|
- **It diagnoses test bugs precisely.** Repeatedly it named the owner's bug before the owner did:
|
||||||
|
- pin-event positions the rules could not produce (v1);
|
||||||
|
- a "growing" conversation that changed the fingerprint (v2);
|
||||||
|
- a 300 KB argv element over Linux's 128 KiB cap (v2);
|
||||||
|
- the limiter release racing the report (v2);
|
||||||
|
- the handler goroutine blocked on the test's own mutex (sift);
|
||||||
|
- a `FreeSlots` rule summing across models (v1).
|
||||||
|
- **It stops correctly when the rules say to.** With a contradiction between protected files it
|
||||||
|
commits only a `stopped` row and reports: v1/01, v2/01 and v2.3/04 were textbook.
|
||||||
|
- **It resumes well.** Given "the previous session did X, do not start over, finish Y", it
|
||||||
|
finishes without redoing work. The pattern: kill by pid, fix the owner's fault, re-copy the
|
||||||
|
given file, start a new session with that prompt.
|
||||||
|
- **Its logs are mostly honest.** It logged its own deviations: the protected-file edit (v1/02),
|
||||||
|
the flush hack (v2.3/02), the example-file edit. The one under-reported row was in v0.
|
||||||
|
- **It is fast when the environment is described.** 6–25 minutes per task once the task text
|
||||||
|
stated the facts it needed.
|
||||||
|
|
||||||
|
## 4. Model failure modes, most frequent first
|
||||||
|
|
||||||
|
1. **Ending the turn after a refused tool call (10 times).** The trigger is always the same: a
|
||||||
|
read or write outside the repository (`/tmp` scratch files, Go's standard library source in
|
||||||
|
the nix store, `/proc/loadavg`, a typo'd path). The OpenCode sandbox refuses it, and the model
|
||||||
|
ends its turn with a plan and no tool call, leaving no commit and no row.
|
||||||
|
- Mitigation that works: `AGENTS.md` says "a refused tool call is not a reason to end the
|
||||||
|
turn; write the experiment as a `_test.go` inside the repository", and the task text states
|
||||||
|
the environment facts the model would otherwise go looking for.
|
||||||
|
- It still happens: the tenth was in sift, after the rule existed.
|
||||||
|
- Budget one restart per few tasks for it. A driver-side auto-resume would recover most of
|
||||||
|
these.
|
||||||
|
2. **Malformed tool call ends the session (2 times).** A stray `</tool_call>` or unparseable
|
||||||
|
call, typically after a long read phase. Restart unchanged; nothing to fix in the task.
|
||||||
|
3. **"Fixing" a timing test in production code (once, severe).** In v2.3 it released every
|
||||||
|
limiter slot at the first flushed byte, to satisfy the owner's racy
|
||||||
|
`InFlight == 0` check. That silently stopped the limiter limiting streaming generation. It did
|
||||||
|
log it as a deviation, but no test caught the consequence.
|
||||||
|
- `AGENTS.md` now forbids changing release, flush or record timing to make a test pass.
|
||||||
|
- Lesson for the owner: a racy given test invites a production-code "fix".
|
||||||
|
4. **Editing protected files when the owner's files conflict (once).** In v1/02 it edited a
|
||||||
|
fixture with a correct change and an honest log, instead of stopping. The rule now says: stop.
|
||||||
|
It has followed that rule since.
|
||||||
|
5. **Rabbit holes on flakiness.** In v1.1 it measured 7/20 failures caused by **its own
|
||||||
|
inference loading the same machine**, then tried to investigate the cause outside the
|
||||||
|
repository and ended on the refusal. The tests passed 12/12 on an idle machine.
|
||||||
|
6. **Oversized tasks loop.** In v1/06 it spent 50 minutes re-reading the same files, and the
|
||||||
|
package never compiled. The same scope split into two tasks went through first time.
|
||||||
|
7. **Small correctness slips the tests did not walk:**
|
||||||
|
- an unchecked type assertion (v0);
|
||||||
|
- a `null` JSON array (v1);
|
||||||
|
- a missing accounting row on one code path (v1);
|
||||||
|
- a duplicated call (v2.3).
|
||||||
|
|
||||||
|
These are the class the review catches, not the tests. It applies a rule where a test looks,
|
||||||
|
and sometimes not everywhere else. The `AGENTS.md` line "when a rule says every, list each
|
||||||
|
place and check them" helped.
|
||||||
|
|
||||||
|
## 5. Owner (customer) faults: where most of the time went
|
||||||
|
|
||||||
|
Ornith's time was lost mostly to the owner's tests and task text. By category:
|
||||||
|
|
||||||
|
- **Racy or timing-fragile given tests** (the most expensive class): a report sent before a
|
||||||
|
deferred release; arrival order resting on sleeps; checking `InFlight` before a deferred
|
||||||
|
release; a mutex held across a request (sift, 55 minutes). These pass on an idle machine and
|
||||||
|
fail under the model's own inference load.
|
||||||
|
- Rule: never assert a value that code settles after the response is sent without waiting for
|
||||||
|
it; never hold a lock across a request the handler needs.
|
||||||
|
- **Tests that contradict the task's own rules:** pin-event positions, the tie-break in spread,
|
||||||
|
the fingerprint-changing "growth", `Pin` on an unseen host. **Walk every given test against the
|
||||||
|
task rules by hand before handover.** The skeleton check proves only that tests compile and
|
||||||
|
fail; it cannot catch a test that no implementation can pass.
|
||||||
|
- **Later tasks invalidating earlier given files** (boxmaker's T19; 5 times):
|
||||||
|
- a new key making an old fixture valid;
|
||||||
|
- a new poller hitting a hit-counting fake;
|
||||||
|
- a new response shape that another plan's test also asserted (v2.3/04, found only because
|
||||||
|
Ornith stopped);
|
||||||
|
- an `example.toml` the task told the model to edit.
|
||||||
|
|
||||||
|
Before handover, grep **all** earlier given files for everything the task changes: strings,
|
||||||
|
JSON shapes, endpoints, counters.
|
||||||
|
- **Given files that fail the gate by themselves:** not `gofmt`-clean; over the 400-line limit;
|
||||||
|
pre-existing gofmt debt in protected directories (sift's guard). Run the gate's own checks on
|
||||||
|
`_files/` and on the protected tree first.
|
||||||
|
- **Missing environment facts:**
|
||||||
|
- `httputil.ReverseProxy` panics with `http.ErrAbortHandler` on client disconnect;
|
||||||
|
- GNU `timeout` exits 124;
|
||||||
|
- Linux's per-argument limit;
|
||||||
|
- router-mode llama-server autoloads on `/props?model=`;
|
||||||
|
- the standard library can't be read from the sandbox.
|
||||||
|
|
||||||
|
The customer describes the world the code runs in. Every missing fact produced either a
|
||||||
|
refusal-ending or a wrong guess.
|
||||||
|
- **Ambiguous or wrong task text:** `<from>>><to>` as a separator (shipped `><`); naming the
|
||||||
|
wrong file; not naming a new file when the package was at its line limit (a 10-minute stall);
|
||||||
|
"does not fail the gate" when `go vet ./...` compiles `main.go`.
|
||||||
|
- **Task sizing:** at most one package per task, with its new files named. The one task that
|
||||||
|
spanned admin, main and wiring failed; split, it passed.
|
||||||
|
|
||||||
|
## 6. Practices that worked (checklist for the next plan)
|
||||||
|
|
||||||
|
1. Acceptance tests first, no reference implementation. Check them against a **panic-only
|
||||||
|
skeleton**: they must compile and fail there for the intended reasons, and every earlier test
|
||||||
|
must stay green.
|
||||||
|
2. **Walk each given test against the task rules and against every other given file**: helpers,
|
||||||
|
fixtures, line limits, `main.go` call sites, response shapes. Run `gofmt -l` and the line
|
||||||
|
check on `_files/`.
|
||||||
|
3. For every earlier given file a new task changes, hand over a **replacement** in `_files/` and
|
||||||
|
list it in the task. Never tell the model to edit a protected file.
|
||||||
|
4. One package per task; name every file to create; say where new code goes when a file is near
|
||||||
|
the limit.
|
||||||
|
5. State the environment facts in the task text: stdlib behaviour, tool exit codes, OS limits,
|
||||||
|
the upstream's real behaviour. Say what cannot be read from the sandbox.
|
||||||
|
6. Tests must not depend on timing under load. Wait on state, never on sleeps, and never check a
|
||||||
|
value settled by a deferred call without waiting for it.
|
||||||
|
7. `AGENTS.md`, standing rules that earned their place:
|
||||||
|
- a refusal is not a reason to end the turn;
|
||||||
|
- never tune production timing to a test;
|
||||||
|
- multi-line commit messages go through `.state/commit-msg.txt` (apostrophes broke `-m`);
|
||||||
|
- stop and report on a protected-file conflict.
|
||||||
|
8. Driver:
|
||||||
|
- one fresh session per task, with the checks listed in §1;
|
||||||
|
- kill a stuck session **by pid** (`pkill -f` matches the driver too);
|
||||||
|
- add a per-session timeout (see §8).
|
||||||
|
9. **Review is not optional.** Two of the worst defects (the missing cancel row; the release on
|
||||||
|
flush) passed every given test. Review with outside probes (kill a host mid-stream, cut a
|
||||||
|
stream, two instances on one database) and `-race -count=3`.
|
||||||
|
10. **Attribution:** have the driver give the model the exact trailer
|
||||||
|
(`Co-Authored-By: ornith-1.5-35b-a3b <ornith-1.5-35b-a3b@llama.cpp.invalid>`) and refuse a
|
||||||
|
commit without it (done in sift's `run-plan.sh`; crossbar's older commits carry
|
||||||
|
`Implemented-By:` and name the model in the log).
|
||||||
|
|
||||||
|
## 7. Environment lessons
|
||||||
|
|
||||||
|
- **The model shares the machine with the tests it runs.** Inference load on straylight made
|
||||||
|
timing-based tests flaky (v1.1), and an unrelated full-suite pytest grew to 60 GB and got
|
||||||
|
killed. Claude Code's memory reaper then killed the driver too. Keep heavy jobs sequential,
|
||||||
|
and cap test processes (`systemd-run --user --scope -p MemoryMax=`).
|
||||||
|
- **The OpenCode sandbox** refuses anything outside the repository, including `/tmp` and the nix
|
||||||
|
store. This is the single biggest source of early endings; design tasks so nothing outside the
|
||||||
|
repository is ever needed.
|
||||||
|
- **OpenCode upgrades** ran a one-time database migration on the next start. One session then
|
||||||
|
sat idle for 46 minutes with no model request, all llama slots idle. A trivial prompt worked
|
||||||
|
afterwards. The driver had no timeout, so nothing noticed.
|
||||||
|
|
||||||
|
## 8. Recommendations
|
||||||
|
|
||||||
|
1. **Auto-resume in the driver:** when a session ends without a commit, a `done` row or a
|
||||||
|
`stopped` row, and the diff is non-empty, start one more session automatically with the
|
||||||
|
standard "do not start over, finish" prompt. Most refusal-endings would then cost minutes,
|
||||||
|
not an owner round-trip.
|
||||||
|
2. **A per-session timeout** in `run-plan.sh` (60–90 min), and a "no model request in 10
|
||||||
|
minutes" watchdog (llama `/slots` idle while OpenCode is running).
|
||||||
|
3. **Keep the owner's pre-handover walk mandatory.** It is the highest-leverage step. Most lost
|
||||||
|
hours trace to a test or a task sentence no one checked against the rules.
|
||||||
|
4. **Put the timing lesson into test design,** not only `AGENTS.md`: given tests should wait on
|
||||||
|
observable state with a deadline (a `waitUntil` helper), never on a sleep or a single read.
|
||||||
|
5. **Keep tasks at one package.** Ornith's quality holds at that size; it degrades sharply beyond.
|
||||||
|
6. Ornith is a good fit for well-specified Go with acceptance tests. It is not yet a fit for
|
||||||
|
work whose spec is "figure out how the environment behaves": give it the facts, or do that
|
||||||
|
part yourself.
|
||||||
Reference in New Issue
Block a user