🔀 Merge gremlins-mvp: gremlins MVP with keep/cut pass and TaskStop check
This commit is contained in:
@@ -0,0 +1,51 @@
|
||||
# Gremlins MVP: unrequested-surface calls
|
||||
|
||||
- Author model: **claude-sonnet-5-5** · Date: 2026-10-17 · Branch `gremlins-mvp` @ `cfeb1bf`
|
||||
- Source: [phase review](2026-10-17-gremlins-mvp-review.md) §2. The user ruled on the rows below: keep the useful ones, cut the rest, one-line reason each. This commit records the calls; the cuts follow in the next commit.
|
||||
|
||||
| # | Item | Call | Reason |
|
||||
|---|---|---|---|
|
||||
| 2 | `briefs/<J>.md`: full prompt per job | keep | Debugging aid. |
|
||||
| 3 | Ledger torn-last-line repair | keep | Protects the ledger after a hard stop. |
|
||||
| 4 | In-process `threading.Lock` around ledger append | cut | Each spawn is its own process, so it guards nothing; the spec says "without locks". |
|
||||
| 5 | `MODEL_RE`, aliases refused at `init` | keep | An alias would change the fingerprint and break cache hits; B1 asks for a full id. |
|
||||
| 6 | `EFFORTS` choices, default `high` | keep | Trivial argparse guard. |
|
||||
| 7 | Tool deny list (`Bash(claude:*)`, `git push/rebase/reset --hard`) | keep | Headless unattended gremlins must not recurse or rewrite history. |
|
||||
| 8 | `SPIRE_GREMLIN_CHILD=1` in the child env | cut | Rider: the refusal it served was dropped, so it is leftover. |
|
||||
| 11 | `refused` / `model_refused` status case | cut | Not in the spec's classify; the harness substitutes unlisted models silently, so `error` covers it. |
|
||||
| 12 | Stream fields tracked and never read (`init`, `msg_ids`, `_ids`, `events`, `turns`) | cut | Never read. |
|
||||
| 13 | `mismatch` in `usage_from_result` | cut | Rider: computed and never read. |
|
||||
| 14 | `wrote_only_5m` | cut | Rider: never called. |
|
||||
| 16 | `NAME_RE` on base/job names | keep | Names become file paths. |
|
||||
| 17 | Note detection by mtime with 2 s slack | cut | Replace with a plain existence check of `notes/<J>.md`, since `spawn` refuses an existing job name (#20). |
|
||||
| 22 | `base_state` in spawn output | cut | `status` reports warmth and `cache` reports the outcome. |
|
||||
| 23 | `init` defaults cwd to the git toplevel | keep | cwd is in the fingerprint; a stable default avoids cache misses. |
|
||||
| 25 | stderr to `raw/<J>.err` | keep | Cheap debugging aid. |
|
||||
| 26 | Reader join timeout 10 s | keep | Without it an orphan holding stdout hangs the helper (spec §7 leftover). |
|
||||
| 28 | Unused `ss=None` injection parameter in `call.py` | cut | Used by no caller. |
|
||||
|
||||
## Result
|
||||
|
||||
Premises checked in the code before each cut. Tests: `uv run --no-project --python 3.12 python -m unittest discover -s tests` gave **24 tests, OK** (run twice).
|
||||
|
||||
| File | Added | Removed |
|
||||
|---|---|---|
|
||||
| `grm/stream.py` | 9 | 39 |
|
||||
| `grm/call.py` | 4 | 4 |
|
||||
| `grm/cli.py` | 2 | 4 |
|
||||
| `grm/launch.py` | 1 | 1 |
|
||||
| **Total** | **16** | **48** |
|
||||
|
||||
Net code: 32 lines removed (16 added, 48 deleted); no test was removed or changed (none existed only for a cut item).
|
||||
|
||||
## Kept against the call
|
||||
|
||||
- **#4 `threading.Lock` around ledger append: kept.** Premise half true: in production each spawn is its own process, so no two threads of one process append. But with the lock removed `test_store.test_parallel_appends_give_whole_lines` (96 of 100 lines) and `test_cli.test_parallel_spawns` (2 of 3 ledger lines) fail, on every run. Appends from separate handles on Windows lose lines. The lock only makes those B3 tests pass in-process; across real processes the same race is unguarded either way (the review's open "Windows append atomicity" item). Cutting it would mean weakening two B3 tests, which the call forbids. Needs a decision: either keep the lock as is, or replace it with a cross-process append (for example a lock file or an `msvcrt` byte-range lock) in a separate change.
|
||||
|
||||
## Notes on the cuts
|
||||
|
||||
- #12: `session_id` from the `init` event is now taken only while unset (`self.session_id is None`), which matches the old "first init wins" behaviour.
|
||||
- #28: `StreamState` is now imported at the top of `call.py` instead of lazily inside the `ss is None` branch.
|
||||
- #17: premise holds. `spawn` refuses an existing `jobs/<J>.json` (`cli.py:255`) and `_execute` writes that file before the call starts, so a note that exists afterwards was written by this job.
|
||||
- #11: nothing outside `stream.py` read `refused` or `model_refused` (code, tests, SKILL.md, templates).
|
||||
- #8, #13, #14, #22: no reader in code, tests, SKILL.md or templates. SKILL.md and templates mentioned none of the cut items, so they are unchanged.
|
||||
@@ -0,0 +1,36 @@
|
||||
# Gremlins B4 live check: TaskStop on a backgrounded spawn
|
||||
|
||||
Date: Sat 2026-10-17 00:13-00:16 (local). Run by: claude-sonnet-5-5. Gremlin model: claude-haiku-5-5 (effort low). Binary 2.1.299, Windows 10 Git Bash.
|
||||
Worktree HEAD e188424. Scratch: throwaway git repo in the session scratchpad `taskstop-check/`.
|
||||
|
||||
## Commands
|
||||
- `G init --run run.md --model claude-haiku-5-5 --effort low --cwd <scratch>`
|
||||
- `G train --run run.md --base b1 --brief train.md` (blocking, ok, $0.0042)
|
||||
- j1: `G spawn ... --job j1` brief "sleep 300": haiku refused (harness blocks a standalone `sleep`), ended ok in 15 s, $0.0019. Not usable for the check.
|
||||
- j2: `G spawn ... --job j2 --brief spawn2.md` (`run_in_background: true`). Brief: run `until [ -e /tmp/never-exists-flag ]; do sleep 2; done`. Heartbeat in raw/j2.jsonl showed the gremlin's Bash running at 30 s.
|
||||
- `TaskStop` on the background call (task btfr2jl7t) at about 00:15:30, ~46 s after spawn. Returned "Successfully stopped".
|
||||
- PIDs found by diffing `Win32_Process` before/after the spawn and following parent links. No process outside this chain was touched.
|
||||
|
||||
## Process tree (j2) and liveness after stop
|
||||
| PID | name | parent | after ~5-10 s | after ~40 s |
|
||||
|---|---|---|---|---|
|
||||
| 22836 / 10324 / 18496 | bash.exe (harness wrappers of the spawn call) | 13436 / 22836 / 10324 | gone | gone |
|
||||
| 1464 | uv.exe | 18496 | gone | gone |
|
||||
| 36336 -> 22192 | python.exe (gremlins.py, 2 procs) | 1464 / 36336 | gone | gone |
|
||||
| 19200 | claude.exe (gremlin, = child_pid in j2.json) | 22192 | gone | gone |
|
||||
| 19580 -> 35212 | bash.exe (gremlin's Bash tool wrappers, 35212 ran the loop) | 19200 / 19580 | gone | gone |
|
||||
| 25392 | conhost.exe | 35212 | gone | gone |
|
||||
| sleep.exe (transient, `sleep 2`, MSYS-forked parent) | - | - | none found | none found |
|
||||
|
||||
No `sleep.exe` and no process with the gremlin's command line existed at either check. Nothing to clean up; no kill issued by me.
|
||||
|
||||
## State after the stop (`run-gremlins/`)
|
||||
- `jobs/j2.json`: `status: running`, `ended: null`, `child_pid: 19200`, no note, no cost. Matches the documented hard-stop behaviour.
|
||||
- `ledger.jsonl`: 2 lines (b1-train, j1). No j2 line. Matches the documented behaviour.
|
||||
- `notes/`: only j1.md; the gremlin wrote no note.
|
||||
|
||||
## Cost
|
||||
About $0.008 total (train 0.0042, j1 0.0019, j2 unrecorded, estimated <= 0.002).
|
||||
|
||||
## Verdict
|
||||
B4 ticked for the claude process and all bash/python/uv wrappers: TaskStop killed the whole tree on Windows Git Bash. Not exercised: the spec §7 orphan case, a long single child (the loop's `sleep 2` children were too short-lived and the loop's bash died with the tree); a `sleep 300` brief is refused by the gremlin's own harness, so that case needs a different long command (for example `python -c "import time; time.sleep(300)"`).
|
||||
@@ -0,0 +1,97 @@
|
||||
# Gremlins MVP: build report
|
||||
|
||||
- Date: 2026-10-16/17 · Orchestrator model: claude-opus-5-5 · Gremlin model: claude-opus-5-5 (all calls)
|
||||
- Branch `gremlins-mvp` (worktree `.claude/worktrees/gremlins-mvp`), from `main` at `b8affd8`. Not pushed, not merged.
|
||||
- Spec: [`2026-10-17-00-gremlins-mvp-spec.md`](../plans/2026-10-17-00-gremlins-mvp-spec.md). Tool: the prototype at `.claude/worktrees/headless-forks/plugins/spire/skills/gremlins/gremlins.py`, unmodified.
|
||||
- Wall time: about 21 minutes, 23:12:58 to 23:33:35, against a 90-minute timebox. Fallback: **not needed**; the prototype did not misbehave once.
|
||||
|
||||
## How it ran
|
||||
|
||||
One base (B1) was trained on the spec, the verdict and the prototype source. It wrote no code. It wrote 70 lines of trunk notes, which pinned every cross-module seam: signatures, return shapes, the state-dir layout, the ledger and record keys, and its own decisions where the spec was open. Then eight gremlins were forked from B1, at most two running at a time (`spawn --mode parallel --paths …`, then `wait --any` / `--all` with Bash timeout 600000):
|
||||
|
||||
1. Wave 1: `stream` + `launch`, then `store` + `call`, each pair started as soon as a slot freed up. Committed in `aff29d6`.
|
||||
2. Wave 2: `cli` + `entry`. `entry` is `gremlins.py` plus SKILL.md. Committed in `9209e7d` and `fd75777`.
|
||||
3. Wave 3: `tests-unit` (started in `entry`'s freed slot, since it does not depend on cli), then `tests-cli`. Committed in `d662790`.
|
||||
4. Integration fix: `79df026`.
|
||||
|
||||
Every gremlin finished `ok` on its first attempt. No gremlin needed a follow-up turn, and none touched another gremlin's files. Each one wrote a note that listed every function with its box.
|
||||
|
||||
## Gremlin metrics (from the prototype's results and ledger)
|
||||
|
||||
| Gremlin | Module | Wall s | Cost delta $ | Cache | Cache read | Lines written |
|
||||
|---|---|---|---|---|---|---|
|
||||
| base B1 (train) | trunk notes only | 407.8 | 1.9725 | n/a | 1,085,902 | 0 code (70 notes) |
|
||||
| stream | `grm/stream.py` (+`__init__.py`) | 104.3 | 0.5175 | hit | 924,301 | 215 |
|
||||
| launch | `grm/launch.py` | 66.7 | 0.4196 | hit | 783,210 | 182 |
|
||||
| store | `grm/store.py` | 62.2 | 0.3815 | hit | 903,383 | 116 |
|
||||
| call | `grm/call.py` | 80.0 | 0.4198 | hit | 908,838 | 86 |
|
||||
| cli | `grm/cli.py` | 230.2 | 1.2211 | hit | 1,667,879 | 359 |
|
||||
| entry | `gremlins.py`, `SKILL.md` | 52.2 | 0.3836 | hit | 916,241 | 14 + 54 |
|
||||
| tests-unit | `tests/test_{store,launch,stream,call}.py` | 80.0 | 0.5441 | hit | 834,361 | 218 |
|
||||
| tests-cli | `tests/fake_claude.py`, `tests/test_cli.py` | 197.3 | 1.1388 | hit | 1,982,799 | 275 |
|
||||
|
||||
- Base cost: $1.9725. The eight spawns together cost $5.0260. Build total (prototype `usage`): **$6.9985**, 9 calls, 0 cold starts, 8 of 8 cache hits.
|
||||
- The live smoke of the MVP cost another $0.2204 (4 calls). That brings the grand total to about **$7.22**.
|
||||
|
||||
## Final line counts against the spec's estimates
|
||||
|
||||
| Module | Lines | Estimate | Reason for the difference |
|
||||
|---|---|---|---|
|
||||
| `grm/store.py` | 116 | 70 | docstrings; `init_state` and `SUBDIRS` added (trunk decision) |
|
||||
| `grm/launch.py` | 182 | 110 | the kept scrub constants and the tool lists; `binary_version` added |
|
||||
| `grm/stream.py` | 215 | 170 | `iso`/`parse_iso` moved here; `TOKEN_KEYS`; `mismatch` kept (4 lines, not used) |
|
||||
| `grm/call.py` | 86 | 60 | feeder and reader threads; the child is killed if `on_start` raises; a second signal escalates to `kill()` |
|
||||
| `grm/cli.py` | 359 | 200 | `delta_header` verbatim (~25); JSON argparse errors (~30); explicit job, ledger and base dicts (~40); status; docstrings |
|
||||
| `gremlins.py` | 14 | (in cli) | |
|
||||
| **Code total** | **972** | **610** | The verdict estimated 600–800. No failure path was dropped, and nothing outside spec §4 was built. |
|
||||
| `tests/` | 493 | about 300 | per-file boilerplate; the fake has 7 scenarios; B4 needs a subprocess and per-platform signalling; one extra test (below) |
|
||||
| `SKILL.md` | 54 | not in the spec | see Deviations |
|
||||
|
||||
## Test and smoke results
|
||||
|
||||
- **Unit tests:** `uv run --no-project --python 3.12 python -m unittest discover -s tests`, run from the skill dir: **24 tests, OK, 7.0 s.** That is 14 module tests and 10 CLI tests, which cover every §6 case from B1 to B8.
|
||||
- **Live smoke** (MVP; real `claude.exe` 2.1.299; scratch git repo; model `claude-opus-5-5`):
|
||||
- `init`: `run.json` was written with the fingerprint, the argv and `fp_hash`, and a frozen preamble copy was made.
|
||||
- `train B1`: `ok`, 5.0 s, $0.0969, session id taken from `--session-id`.
|
||||
- `spawn s1`: `ok`, **`cache: hit`** (69,651 tokens read, 1,664 written), 36.5 s, cost delta $0.0431, `base_state: warm`. The note was written and the repo was left untouched.
|
||||
- The ledger had **two lines**, each with `model`, `models_used`, six token columns, `cost_delta_usd`, `cost_cumulative_usd` and `hit_at`.
|
||||
- `status`: B1 `warm`, `ttl_s` 3600, `ttl_left_s` 3566.
|
||||
- **Parallel test (B3):** two `spawn`s, p1 and p2, ran as separate Bash `run_in_background` calls. Both started at 23:30:33 and ended at 23:30:44 and 23:30:45. Both completion notices arrived with the result JSON and exit 0, and both reported `cache: hit`. The run ended with two new job files, two new whole ledger lines (four in total) and two notes.
|
||||
- **Processes afterwards:** `Get-CimInstance Win32_Process` shows no `claude -p`, `python` or `uv` process left. The only `claude.exe` processes are the Desktop app and its own sessions. The prototype's `list` showed nothing running, and `expire --all` retired its base B1, so no keep-warm ping can fire later.
|
||||
- **Not run live:** the B4 `TaskStop` on a backgrounded `spawn` (spec §5 step 4), a B7 `rebase`, and the B8 gate on WSL. These paths are covered only by the fake-claude tests: CTRL_BREAK to the helper subprocess, and rebase argv and session id.
|
||||
|
||||
## Deviations from the spec, each with its reason
|
||||
|
||||
1. **`SKILL.md` was added**, although it is not in the module table. Reason: B3 and B4 only work if the orchestrator is told to run `spawn` via `run_in_background` and to stop it with `TaskStop`, and the skill file is where it is told.
|
||||
2. **`templates/rebase.md` was reworded** ("a fork … becomes the new base `${new_base}` (phase …)" became "base `${base}` itself, resumed"). cli stopped passing `new_base` and `phase`. Reason: §3 said to use the templates as is, but the old text contradicts B7, which resumes without a fork and keeps the same base. The cli gremlin flagged it, and the fix is in `79df026`.
|
||||
3. **`spawn.md`:** `${commit_rule}` became the fixed line "Commit only if the brief below says so", and `${mode}` was removed. This is per spec §3; the fixed line is so that the rule is still visible.
|
||||
4. **Trunk-note decisions where the spec was open:**
|
||||
- `build_fingerprint` takes `mcp_config` and returns `{fingerprint, argv, fp_hash}`.
|
||||
- The argv builders take the stored run dict, so that a test's fake `argv` is honoured.
|
||||
- `resume_argv` has no `--session-id`, because claude refuses it on a resume without a fork.
|
||||
- The fingerprint adds `allowed_tools` and `disallowed_tools`, because `dontAsk` denies every tool that is not allowed.
|
||||
- `resolve_binary` returns a str, not the prototype's tuple.
|
||||
5. **`models_used` is ordered by usage, largest first**, not by name. Reason: by name, a haiku side-call could become `model` (= `models_used[0]`).
|
||||
6. **The ledger line adds `hit_at`.** Reason: B6 derives warmth from the ledger, and the line's `ts` is the end of the call, not the time the cache was read.
|
||||
7. **Job ids:**
|
||||
- train is `<B>-train`; it can be re-run only while there is no base file.
|
||||
- rebase is `<B>-rebase<n>`; a failed rebase re-run overwrites its own job and raw files.
|
||||
- spawn takes `--job J`, and an existing J is refused.
|
||||
8. **The base's `cost_cumulative_usd` is stored unrounded.** Ledger and printed costs are still rounded once, at 6 decimals. Reason: a rounded basis made the deltas drift by 1e-6 (0.020001 instead of 0.02).
|
||||
9. **`StreamState` prefers the result's `session_id` over the init event's.** Reason: spec §7, B7.
|
||||
10. **Exit codes are 0 / 1 / 2:** 0 ok, 1 a call that ended `error` or `stopped`, 2 refused. `status` refuses a run with no `run.json`.
|
||||
11. **No `SPIRE_GREMLIN_CHILD` refusal.** Reason: no box needs it, and it would break the helper's own tests when they run inside a gremlin.
|
||||
12. **One extra test**, `test_call.test_tee_and_parse`. Reason: it is the only check that the prompt goes in on stdin as UTF-8 and that the stream is teed to the raw file and parsed.
|
||||
|
||||
## Observations on the dogfooding
|
||||
|
||||
- The prototype raised `rebase_due: phase_boundary` on every wave-2 and wave-3 spawn, because `--phase` was used as a wave label. The flag is advisory, so it was ignored. The delta header gave the forks the new commits instead.
|
||||
- The trunk notes were what made the parallel work fit together: four modules written blind against each other imported cleanly on the first try. The only integration fix was the template wording.
|
||||
- Each fork read 0.8 to 2.0 M tokens from cache, at about $0.38 to $1.22 per module. The base's $1.97 was spent once.
|
||||
|
||||
## Open items
|
||||
|
||||
- Live B4 check: a `TaskStop` on a backgrounded MVP `spawn`.
|
||||
- Live B7 check: does a plain `--resume` report a cumulative `total_cost_usd`? The cost basis assumes it does.
|
||||
- The B8 gate on WSL.
|
||||
- Cross-process ledger appends on Windows are not strictly atomic (CRT seek + write). This is accepted per the trunk notes. The live parallel run gave whole lines.
|
||||
@@ -0,0 +1,82 @@
|
||||
# Gremlins MVP: phase review
|
||||
|
||||
- Reviewer model: **claude-opus-5-5[1m]** · Date: 2026-10-16 · Branch `gremlins-mvp` @ `0493f03` · Timebox 15 min
|
||||
- Scope: `plugins/spire/skills/gremlins/` against the [spec](../plans/2026-10-17-00-gremlins-mvp-spec.md) and the [build report](2026-10-17-gremlins-mvp-build.md)
|
||||
- Gate: `uv run --no-project --python 3.12 python -m unittest discover -s tests` gave **24 tests, OK, 7.0 s**, the same as the build report. No live `claude` calls in this review.
|
||||
|
||||
## 1. Boxes
|
||||
|
||||
| Box | Verdict | Evidence | Verified by |
|
||||
|---|---|---|---|
|
||||
| B1 | ticked | `cli.py:226-247` train with `--session-id` (`launch.py:172`); fingerprint stored once in `run.json`, overwrite refused `cli.py:202`; `launch.py:139-169` | live smoke + `test_cli` happy/missing_binary/train_failures, `test_stream` |
|
||||
| B2 | ticked | `launch.py:176-177` stored argv + `--resume --fork-session --session-id`; `cli.py:250-269`; `cache_verdict` `stream.py:178` | live smoke (`cache: hit`) + `test_launch`, `test_cli` happy/unknown_base/cache_miss |
|
||||
| B3 | ticked | no detach/supervisor code anywhere; one job file per job `cli.py:134`; ledger append `store.py:85` | live (two `run_in_background` spawns) + `test_parallel_spawns`, `test_store` |
|
||||
| B4 | partly | no `kill` command; handler `call.py:53-66` (SIGTERM/SIGINT/SIGBREAK, second signal kills) | unit only (`test_call` SIGTERM, `test_cli` CTRL_BREAK). Spec §5 step 4 live `TaskStop` on an MVP spawn **not run** |
|
||||
| B5 | ticked | `stream.py:141-175` (models by usage, six token columns, delta rounded once); ledger line `cli.py:174-181` | live smoke (two ledger lines) + `test_stream`, `test_usage_models_and_cost` |
|
||||
| B6 | ticked | `_warmth` `cli.py:119-126` derived from ledger `hit_at`; `stream.py:206-215`; no ping code | live `status` (warm, ttl_left 3566) + `test_stream` cold/5m/expiring, `test_status_cold_without_hit` |
|
||||
| B7 | partly | `cli.py:272-298`, `launch.py:180-182` (no fork, no `--session-id`), session id from result `cli.py:294`, running-jobs warning `cli.py:278-280` | unit only (fake). Not run live; the cumulative-cost basis after a plain resume is an open assumption |
|
||||
| B8 | partly | only portable `Popen.terminate/kill` + `getattr(signal,"SIGBREAK")` `call.py:11`; scrub `launch.py:88-103` | `test_launch` scrub test. Spec §5 step 6 non-Windows gate **not run** |
|
||||
|
||||
## 2. Unrequested surface (list only; J = the build report justifies it)
|
||||
|
||||
| # | Where | What | J |
|
||||
|---|---|---|---|
|
||||
| 1 | `store.py:10,25-30` | `SUBDIRS` + `init_state` | J |
|
||||
| 2 | `store.py:10`, `cli.py:136` | `briefs/<J>.md`: full prompt written per job | - |
|
||||
| 3 | `store.py:91-97` | ledger torn-last-line repair before append | - |
|
||||
| 4 | `store.py:14,98` | in-process `threading.Lock` around ledger append (spec: "without locks") | - |
|
||||
| 5 | `launch.py:10`, `cli.py:211` | `MODEL_RE`: aliases refused at `init` | - |
|
||||
| 6 | `launch.py:11`, `cli.py:330` | `EFFORTS` choices; default effort `high` | - |
|
||||
| 7 | `launch.py:13-15,158-161` | fixed tool list, `allowed_tools`, `disallowed_tools` (incl. `Bash(claude:*)`, `git push/rebase/reset --hard`) | J (fields); deny list itself not |
|
||||
| 8 | `launch.py:41` | `SPIRE_GREMLIN_CHILD=1` still pinned into the child env although the refusal was dropped | partly (dev. 11) |
|
||||
| 9 | `launch.py:36-40,65` | `SCRUB_KEEP`/`"keep"` class: no behavioural effect (keep and unlisted both pass) | ported per §3 |
|
||||
| 10 | `stream.py:13-27` | `iso`/`parse_iso` in stream | J |
|
||||
| 11 | `stream.py:43,56-58,127-128` | `refused` + `model_refused` status case (not in spec's `classify` tuple) | - |
|
||||
| 12 | `stream.py:34,36-37,44,73-75` | `init`, `msg_ids`, `_ids`, `events`, `turns`: tracked, never read | - |
|
||||
| 13 | `stream.py:165-167,174` | `mismatch` computed, never read | J |
|
||||
| 14 | `stream.py:200-203` | `wrote_only_5m`: listed in §3, never called | (spec) |
|
||||
| 15 | `cli.py:180` | ledger field `hit_at` | J |
|
||||
| 16 | `cli.py:16,87-90` | `NAME_RE` validation of base/job names | - |
|
||||
| 17 | `cli.py:17,163-167` | note detection by file mtime with 2 s slack | - |
|
||||
| 18 | `cli.py:22-32,355-357` | `Refusal` / JSON argparse errors; `ValueError`/`OSError` mapped to exit 2 | J |
|
||||
| 19 | `cli.py:359` | exit codes 0/1/2 | J |
|
||||
| 20 | `cli.py:231,255` | train refuses existing base; spawn refuses existing job | J |
|
||||
| 21 | `cli.py:281,291-293` | rebase job id `<B>-rebase<n>`; `rebases[]` entries `{job, at, from_commit, from_session}` | J (ids) |
|
||||
| 22 | `cli.py:258,269` | spawn output `base_state`, computed before the call | - |
|
||||
| 23 | `cli.py:208` | `init` defaults cwd to the git toplevel | - |
|
||||
| 24 | `cli.py:214-217` | frozen preamble copy + `mcp-empty.json` in the state dir | J (mcp) |
|
||||
| 25 | `call.py:48` | stderr to `raw/<J>.err` | - |
|
||||
| 26 | `call.py:10,83-85` | reader join timeout 10 s (orphan holding stdout) | - |
|
||||
| 27 | `call.py:56-57` | second signal escalates to `kill()` | J |
|
||||
| 28 | `call.py:42` | `ss=None` injection parameter, used by no caller | - |
|
||||
| 29 | `SKILL.md` | the skill file | J |
|
||||
| 30 | `tests/test_call.py:24` | `test_tee_and_parse` | J |
|
||||
|
||||
**30 items, 15 justified (J or partly) by the build report, 15 not.** None looks like a bug. Most are a few lines; #2, #3, #11, #12, #17 are the ones that add real behaviour.
|
||||
|
||||
## 3. The 972 vs 610 gap
|
||||
|
||||
Measured with `ast` (docstrings), blank, comment and import lines:
|
||||
|
||||
| Module | Est. | Actual | Blank | Doc | Cmt | Import | Logic | Where the logic surplus goes |
|
||||
|---|---|---|---|---|---|---|---|---|
|
||||
| `store.py` | 70 | 116 | 23 | 8 | 0 | 7 | 78 | `init_state`, torn-line repair, lock (#1, #3, #4) |
|
||||
| `launch.py` | 110 | 182 | 31 | 9 | 6 | 7 | 129 | scrub constants as estimated; plus tool lists, `binary_version`, `build_fingerprint` re-validating what `cmd_init` already checks (`launch.py:141-150` vs `cli.py:205-212`) |
|
||||
| `stream.py` | 170 | 215 | 28 | 17 | 0 | 4 | 166 | on estimate; dead `wrote_only_5m`/`turns`/`mismatch` (~12) offset by a compact `classify` |
|
||||
| `call.py` | 60 | 86 | 9 | 4 | 0 | 6 | 67 | two threads, kill escalation, join timeout |
|
||||
| `cli.py` | 200 | 359 | 52 | 15 | 0 | 11 | 281 | multi-line dict literals for job/ledger/base (~35); `delta_header` + git helpers (~35, spec counted it "as is"); `_execute` hit/note/excerpt logic (~15); duplicated init validation (~8); `Refusal`/`_Parser` (~12, not the ~30 the build report says) |
|
||||
| `gremlins.py` | (cli) | 14 | 3 | 1 | 4 | 3 | 3 | PEP 723 header |
|
||||
| **Total** | **610** | **972** | 146 | 54 | 10 | 38 | **724** | |
|
||||
|
||||
Form (blank, docstring, comment, import) is **248** of the 362 extra lines. Logic is 724 against 610, so +114 (+19%), and 81 of that is in `cli.py`. That surplus is mostly verbose layout (dicts written over several lines, duplicated validation) plus the small additions in §2. Only `call.py`'s threads and `_execute`'s hit/note logic are real logic the spec underestimated. **Verdict: mostly form.** The substance is within the verdict's 600–800 band.
|
||||
|
||||
## Handoff flags answered
|
||||
|
||||
- Dev. 1 (SKILL.md): accepted; B3/B4 need the orchestrator to be told. Dev. 2 (rebase.md wording): correct, the old text contradicted B7. Dev. 3: per §3.
|
||||
- Dev. 4–6, 8, 9: consistent with the code and tests (`test_cli` happy checks the unrounded basis to 6 places). Dev. 7, 10: as implemented. Dev. 11: the refusal is gone but the env var is still set (#8). Dev. 12: justified, and the only test that covers stdin and the tee.
|
||||
- Open items (live B4/B7/B8, Windows append atomicity): still open. These three boxes are "partly", not failures.
|
||||
- Weakened checks / tests that cannot fail: none found. On Windows `test_stop_signal` sends CTRL_BREAK to a process group that includes the fake child, so it cannot tell the helper's `terminate()` apart from the child dying on its own. `test_call.test_sigterm_stops_child` (in-process `raise_signal`) does isolate the handler path.
|
||||
|
||||
## Bug (one line)
|
||||
|
||||
No real bug noticed. The nearest is harmless duplication: `init` validates model, binary and cwd twice (`cli.py:205-212`, `launch.py:141-150`).
|
||||
Reference in New Issue
Block a user