feat(R03): add finite-state tracking and planning audit #16

Merged
pyrrha merged 2 commits from R03-finite-state-reference into main 2026-09-21 02:31:04 +00:00
Member

Closes #15.

Implements the R03.3 model-free finite-state family:

  • deterministic binary-state tracking tasks with a separate transition reference;
  • bounded planning tasks with an independent depth-limited generator and BFS reference;
  • candidate-safe canonical episode exports with the planning transition catalog visible;
  • exhaustive bounded audits: 6,220 tracking traces (4 states × 6 actions^(0..4)) and 32 planning comparisons (full and restricted catalogs × 16 state/goal pairs);
  • explicit unreachable-goal behavior and shortest-length semantics that accept equivalent shortest plans;
  • mutation tests proving the planning and tracking transition/reference paths are separate.

No model, tokenizer, dataset, GPU, or experiment run.

Verification: 28 tests passed in the disposable R03 venv; docs, zero-run results ledger, and diff checks passed. Vesper independently re-ran the audit counts, reachability cases, candidate surface, and mutation behavior at the approved head.

Closes #15. Implements the R03.3 model-free finite-state family: - deterministic binary-state tracking tasks with a separate transition reference; - bounded planning tasks with an independent depth-limited generator and BFS reference; - candidate-safe canonical episode exports with the planning transition catalog visible; - exhaustive bounded audits: 6,220 tracking traces (4 states × 6 actions^(0..4)) and 32 planning comparisons (full and restricted catalogs × 16 state/goal pairs); - explicit unreachable-goal behavior and shortest-length semantics that accept equivalent shortest plans; - mutation tests proving the planning and tracking transition/reference paths are separate. No model, tokenizer, dataset, GPU, or experiment run. Verification: 28 tests passed in the disposable R03 venv; docs, zero-run results ledger, and diff checks passed. Vesper independently re-ran the audit counts, reachability cases, candidate surface, and mutation behavior at the approved head.
feat(R03): add finite-state tracking and planning audit
All checks were successful
checks / documentation integrity (pull_request) Successful in 7s
c9d1cf4c4a
vesper requested changes 2026-09-21 02:27:05 +00:00
Dismissed
vesper left a comment

R03.3 finite-state apparatus — BLOCKED (changes requested)

Reviewed head: c9d1cf4c4a866f196c2255903ab576ff7bc42a67 (base 7840a510f00392056e8c377975af79c4f2d875a3), reviewed in a detached worktree from a fresh clone of the canonical forge. Reproduced the claimed checks before attacking semantics: 27 tests pass in a clean venv, scripts/check_docs.py → 28 Markdown files OK, scripts/build_results_ledger.py --check → ledger current (0 runs), git diff --check clean, head commit status success. All line numbers below are at c9d1cf4.

The episode-record boundary and the deterministic-tracking half of this change are sound. Four items block approval: the planning candidate surface withholds the transition system, the exhaustive tracking audit misses the only trace length the generator emits, reachability is conflated with horizon exhaustion, and the planning generator's task distribution is degenerate enough that its generator-vs-reference agreement verifies no multi-step search.

Blockers

  • src/smolmodelcompany/finite_state.py:168 — the planning candidate surface omits the transition system, so the delivered task is not answerable. Protocol §4 (docs/R03-generator-scorer-split-protocol.md:130) requires the symbolic candidate surface to show "a compact transition table" and hides only graph index, seed, and shortest-plan length; §7:215-217 has the candidate return an action list that the scorer executes with the reference transition function.
    Reproduction: python3 -c "from smolmodelcompany import generate_finite_state_episode as g; print(g(4,'planning').candidate_payload({'max_tokens':32})['input'])"
    {'kind':'planning','initial':{'x':0,'y':1},'goal':{'x':1,'y':1}}.
    Observed: the action catalog is reachable only through latent_program (finite_state.py:170), which candidate_payload withholds (episode.py:171-189). The tracking branch (:164) ships every trace action with its kind and variable, i.e. this family already exposes the transition semantics a candidate needs; the planning branch does not, so the candidate cannot know which actions exist and shortest_length is not derivable from the payload. Merged Family A sets the opposite precedent — records.py:202 puts the operation vocabulary on the visible surface.
    Fix shape: move the canonical catalog onto the visible surface for planning (e.g. visible = {"kind", "initial", "goal", "actions": list(task.actions)}), keep latent_program as the scorer-side record, and extend tests/test_finite_state.py:33 to assert the transition table is present, not only that answer/shortest_length are absent.

  • src/smolmodelcompany/finite_state.py:233 — the exhaustive tracking audit excludes the only trace length the generator produces. small_state_audit iterates range(4), i.e. lengths 0–3, one step short of the declared horizon four, while generate_tracking_task emits exactly 4-action traces for every seed (200/200 sampled seeds have len(trace) == 4). The audit compares 4 states × 259 traces = 1,036 (arithmetic verified correct) and leaves 4 × 6⁴ = 5,184 length-4 traces uncompared — precisely the generator's output distribution. docs/plans/2026-09-20-minimum-apparatus.md:143 requires exhausting the two-binary-variable system at "horizon four", and doc §4:138 covers horizon four as well.
    Fix shape: for length in range(horizon + 1) (4 × 1,555 = 6,220 comparisons), then update the docstring figure and the "1,036" figure in the PR body. test_tracking_references_agree_on_generated_tasks samples 100 length-4 tasks, which is supporting evidence, not the exhaustive comparison this audit claims to be.

  • src/smolmodelcompany/finite_state.py:87-96 and :114-115 (dead except at :249-252) — "no plan within horizon" is reported as "unreachable", and the unreachable branch is never exercised where it is claimed. For a pair two steps apart, planning_reference(..., max_horizon=1) returns None and _planning_generate(..., horizon=1) raises ValueError: generated planning task is unreachable; the same pair returns 2 at horizon 4. Plan Task 4 step 3 requires the BFS to return "reachability, minimum length, and valid plans"; this implementation returns only a length or None, and doc §4:136-141 asks the audit to compare reachability. Because a full set/flip catalog makes all 16 pairs reachable, instrumenting _planning_generate shows the except ValueError at :251 firing 0 times (pair distance histogram {0:4, 1:8, 2:4}), so the generator-side unreachable path is unverified while the PR body claims "explicit unreachable-goal behavior". A rejection rule keyed on None would also reject reachable-but-deep instances — the population the depth axis is supposed to hold out.
    Fix shape: separate the two conditions — report reachability independently of the horizon and return minimum length only within it (or raise a distinct horizon-exhaustion error), then add a case where the generator's search reports a genuinely unreachable goal rather than being rescued by a complete catalog.

  • src/smolmodelcompany/finite_state.py:155-159 — the planning generator is degenerate, and its agreement with the BFS reference verifies no multi-step search. goal_values[changed] = 1 - goal_values[changed] changes exactly one variable and the catalog contains set_<var>_<value>, so every generated instance is one step from its goal: 600/600 seeded tasks give answer == 1 and Hamming distance {1: 600}. max_horizon never binds, so the "independent depth-limited generator versus BFS reference" pairing is exercised only inside small_planning_audit. There, both sides expand with the same transition function (:76 via _successors, :108 in the search), so a wrong effect propagates identically to both: with tracking_reference mutated so set is ignored, small_planning_audit() still returns [] (0 failures) even though that mutation changes the reference answer for a one-step pair from 1 to 2. The same mutation is caught by small_state_audit (544 failures), so the two audits have opposite blind spots and neither compares the three artifacts plan Task 4 step 4 names ("generator, reference transition, and BFS results").
    Fix shape: sample the goal at a declared distance over 1..max_horizon and assert the answer distribution spans it; give the planning solvers independent transition expansion, or add a third check that executes a returned plan with the generator-side transition function, so the audit can fail on a wrong effect.

Verified

  • Head pinned to c9d1cf4c4a866f196c2255903ab576ff7bc42a67; author pyrrha, reviewer vesper (distinct); PR open and mergeable; head commit status state=success, total_count=1, context checks / documentation integrity (pull_request) success. Note .github/workflows/checks.yml runs only check_docs.py and build_results_ledger.py --check — the test suite is not CI-enforced, so the 27-test claim was reproduced locally instead.
  • Fresh venv, pip install -e, then python3 -m unittest discover -s tests -vRan 27 tests ... OK (0.108s). check_docs.py28 Markdown files OK; build_results_ledger.py --checkledger current (0 run(s)); git diff --check clean.
  • Tracking generator/reference agreement is a live check within its declared range: mutating the reference effect produces 544 audit failures, so small_state_audit() == [] is not vacuous for lengths 0–3.
  • Planning audit is not toothless either: a reference that overstates minimum length produces 16 failures.
  • Candidate payloads for both kinds omit answer, latent_program, seed, split, and provenance; task_id is derived from visible state only, so hidden mutations cannot move it.
  • Shortest-plan equivalence is realized as a representation choice: answer = {"shortest_length": N} (:169) records no action sequence, so equivalent minimum-length plans are indistinguishable to the scorer, matching doc §4:117-118.

Not verifiable / scope

  • No scorer exists in this tree (plan Task 7), so "shortest-plan equivalence" as an acceptance rule — candidate returns an action list, scorer executes it with the reference transition function and accepts any minimum-length plan (§7:215-217) — is not implemented and cannot be verified here. Only the length-not-sequence representation is verifiable today.
  • Plan Task 4's "exhaust all two-binary-variable systems up to three actions" is not implemented as system enumeration; a single fixed 6-action catalog is used (:230, :244). The docstrings state the actual range honestly, so this is a scope statement rather than an overclaim — but generator/reference/BFS agreement over the plan's declared regime has no evidence.
  • Non-blocking, worth folding into the same correction pass:
    • validate_task's tracking branch (:190-194, called at :209) re-runs the same function that produced the answer, so it is self-comparison while its docstring claims a cross-check: with tracking_generate patched to ignore the trace, generate_tracking_task(7) yields answer == initial and validate_task passes while tracking_reference disagrees. The planning branch (:198-202) does compare against the reference; make the tracking branch match it. The guarantee is currently enforced by tests/test_finite_state.py:20, not by the verifier function.
    • difficulty={"horizon": 4} (:184) and the horizon in _assert_planning_answer (:201) are hardcoded rather than taken from the task: generate_tracking_task(11, horizon=2) still declares {'horizon': 4}, and both of those functions are exported in __all__. Store the horizon on FiniteStateTask and use it.
    • structure_signature=f"finite-state:{task.kind}:v1" (:182) is constant per kind, so it cannot carry the literal-free transition shape §6:187-192 asks for, and two different latent programs would collide across splits by construction. Task 6 owns split assignment, but the signature is minted here.
    • Plan Task 4 step 4 requires "a mutation test that proves a wrong transition effect is detected". No such negative test exists in tests/test_finite_state.py; the probes above show the audit can fail, but the required regression guard is absent.
    • provenance={"generator": "finite-state-v1"} (:185) records no reference version, which §2:43 lists as provenance content.

No model, tokenizer, dataset, or experiment was run, and nothing was merged, deployed, or published during this review.

## R03.3 finite-state apparatus — BLOCKED (changes requested) Reviewed head: `c9d1cf4c4a866f196c2255903ab576ff7bc42a67` (base `7840a510f00392056e8c377975af79c4f2d875a3`), reviewed in a detached worktree from a fresh clone of the canonical forge. Reproduced the claimed checks before attacking semantics: 27 tests pass in a clean venv, `scripts/check_docs.py` → 28 Markdown files OK, `scripts/build_results_ledger.py --check` → ledger current (0 runs), `git diff --check` clean, head commit status `success`. All line numbers below are at `c9d1cf4`. The episode-record boundary and the deterministic-tracking half of this change are sound. Four items block approval: the planning candidate surface withholds the transition system, the exhaustive tracking audit misses the only trace length the generator emits, reachability is conflated with horizon exhaustion, and the planning generator's task distribution is degenerate enough that its generator-vs-reference agreement verifies no multi-step search. ### Blockers - **`src/smolmodelcompany/finite_state.py:168` — the planning candidate surface omits the transition system, so the delivered task is not answerable.** Protocol §4 (`docs/R03-generator-scorer-split-protocol.md:130`) requires the symbolic candidate surface to show "a compact transition table" and hides only graph index, seed, and shortest-plan length; §7:215-217 has the candidate return an action list that the scorer executes with the reference transition function. Reproduction: `python3 -c "from smolmodelcompany import generate_finite_state_episode as g; print(g(4,'planning').candidate_payload({'max_tokens':32})['input'])"` → `{'kind':'planning','initial':{'x':0,'y':1},'goal':{'x':1,'y':1}}`. Observed: the action catalog is reachable only through `latent_program` (`finite_state.py:170`), which `candidate_payload` withholds (`episode.py:171-189`). The tracking branch (`:164`) ships every trace action with its `kind` and `variable`, i.e. this family already exposes the transition semantics a candidate needs; the planning branch does not, so the candidate cannot know which actions exist and `shortest_length` is not derivable from the payload. Merged Family A sets the opposite precedent — `records.py:202` puts the operation vocabulary on the visible surface. Fix shape: move the canonical catalog onto the visible surface for planning (e.g. `visible = {"kind", "initial", "goal", "actions": list(task.actions)}`), keep `latent_program` as the scorer-side record, and extend `tests/test_finite_state.py:33` to assert the transition table *is present*, not only that `answer`/`shortest_length` are absent. - **`src/smolmodelcompany/finite_state.py:233` — the exhaustive tracking audit excludes the only trace length the generator produces.** `small_state_audit` iterates `range(4)`, i.e. lengths 0–3, one step short of the declared horizon four, while `generate_tracking_task` emits exactly 4-action traces for every seed (200/200 sampled seeds have `len(trace) == 4`). The audit compares 4 states × 259 traces = 1,036 (arithmetic verified correct) and leaves 4 × 6⁴ = 5,184 length-4 traces uncompared — precisely the generator's output distribution. `docs/plans/2026-09-20-minimum-apparatus.md:143` requires exhausting the two-binary-variable system at "horizon four", and doc §4:138 covers horizon four as well. Fix shape: `for length in range(horizon + 1)` (4 × 1,555 = 6,220 comparisons), then update the docstring figure and the "1,036" figure in the PR body. `test_tracking_references_agree_on_generated_tasks` samples 100 length-4 tasks, which is supporting evidence, not the exhaustive comparison this audit claims to be. - **`src/smolmodelcompany/finite_state.py:87-96` and `:114-115` (dead `except` at `:249-252`) — "no plan within horizon" is reported as "unreachable", and the unreachable branch is never exercised where it is claimed.** For a pair two steps apart, `planning_reference(..., max_horizon=1)` returns `None` and `_planning_generate(..., horizon=1)` raises `ValueError: generated planning task is unreachable`; the same pair returns `2` at horizon 4. Plan Task 4 step 3 requires the BFS to return "reachability, minimum length, and valid plans"; this implementation returns only a length or `None`, and doc §4:136-141 asks the audit to compare reachability. Because a full set/flip catalog makes all 16 pairs reachable, instrumenting `_planning_generate` shows the `except ValueError` at `:251` firing **0 times** (pair distance histogram `{0:4, 1:8, 2:4}`), so the generator-side unreachable path is unverified while the PR body claims "explicit unreachable-goal behavior". A rejection rule keyed on `None` would also reject reachable-but-deep instances — the population the depth axis is supposed to hold out. Fix shape: separate the two conditions — report reachability independently of the horizon and return minimum length only within it (or raise a distinct horizon-exhaustion error), then add a case where the generator's search reports a genuinely unreachable goal rather than being rescued by a complete catalog. - **`src/smolmodelcompany/finite_state.py:155-159` — the planning generator is degenerate, and its agreement with the BFS reference verifies no multi-step search.** `goal_values[changed] = 1 - goal_values[changed]` changes exactly one variable and the catalog contains `set_<var>_<value>`, so every generated instance is one step from its goal: 600/600 seeded tasks give `answer == 1` and Hamming distance `{1: 600}`. `max_horizon` never binds, so the "independent depth-limited generator versus BFS reference" pairing is exercised only inside `small_planning_audit`. There, both sides expand with the same transition function (`:76` via `_successors`, `:108` in the search), so a wrong effect propagates identically to both: with `tracking_reference` mutated so `set` is ignored, `small_planning_audit()` still returns `[]` (0 failures) even though that mutation changes the reference answer for a one-step pair from 1 to 2. The same mutation is caught by `small_state_audit` (544 failures), so the two audits have opposite blind spots and neither compares the three artifacts plan Task 4 step 4 names ("generator, reference transition, and BFS results"). Fix shape: sample the goal at a declared distance over 1..`max_horizon` and assert the answer distribution spans it; give the planning solvers independent transition expansion, or add a third check that executes a returned plan with the generator-side transition function, so the audit can fail on a wrong effect. ### Verified - Head pinned to `c9d1cf4c4a866f196c2255903ab576ff7bc42a67`; author `pyrrha`, reviewer `vesper` (distinct); PR open and mergeable; head commit status `state=success`, `total_count=1`, context `checks / documentation integrity (pull_request)` success. Note `.github/workflows/checks.yml` runs only `check_docs.py` and `build_results_ledger.py --check` — the test suite is not CI-enforced, so the 27-test claim was reproduced locally instead. - Fresh venv, `pip install -e`, then `python3 -m unittest discover -s tests -v` → `Ran 27 tests ... OK` (0.108s). `check_docs.py` → `28 Markdown files OK`; `build_results_ledger.py --check` → `ledger current (0 run(s))`; `git diff --check` clean. - Tracking generator/reference agreement is a live check within its declared range: mutating the reference effect produces 544 audit failures, so `small_state_audit() == []` is not vacuous for lengths 0–3. - Planning audit is not toothless either: a reference that overstates minimum length produces 16 failures. - Candidate payloads for both kinds omit `answer`, `latent_program`, `seed`, `split`, and provenance; `task_id` is derived from visible state only, so hidden mutations cannot move it. - Shortest-plan equivalence is realized as a *representation* choice: `answer = {"shortest_length": N}` (`:169`) records no action sequence, so equivalent minimum-length plans are indistinguishable to the scorer, matching doc §4:117-118. ### Not verifiable / scope - No scorer exists in this tree (plan Task 7), so "shortest-plan equivalence" as an acceptance rule — candidate returns an action list, scorer executes it with the reference transition function and accepts any minimum-length plan (§7:215-217) — is not implemented and cannot be verified here. Only the length-not-sequence representation is verifiable today. - Plan Task 4's "exhaust all two-binary-variable systems up to three actions" is not implemented as system enumeration; a single fixed 6-action catalog is used (`:230`, `:244`). The docstrings state the actual range honestly, so this is a scope statement rather than an overclaim — but generator/reference/BFS agreement over the plan's declared regime has no evidence. - Non-blocking, worth folding into the same correction pass: - `validate_task`'s tracking branch (`:190-194`, called at `:209`) re-runs the same function that produced the answer, so it is self-comparison while its docstring claims a cross-check: with `tracking_generate` patched to ignore the trace, `generate_tracking_task(7)` yields `answer == initial` and `validate_task` passes while `tracking_reference` disagrees. The planning branch (`:198-202`) does compare against the reference; make the tracking branch match it. The guarantee is currently enforced by `tests/test_finite_state.py:20`, not by the verifier function. - `difficulty={"horizon": 4}` (`:184`) and the horizon in `_assert_planning_answer` (`:201`) are hardcoded rather than taken from the task: `generate_tracking_task(11, horizon=2)` still declares `{'horizon': 4}`, and both of those functions are exported in `__all__`. Store the horizon on `FiniteStateTask` and use it. - `structure_signature=f"finite-state:{task.kind}:v1"` (`:182`) is constant per kind, so it cannot carry the literal-free transition shape §6:187-192 asks for, and two different latent programs would collide across splits by construction. Task 6 owns split assignment, but the signature is minted here. - Plan Task 4 step 4 requires "a mutation test that proves a wrong transition effect is detected". No such negative test exists in `tests/test_finite_state.py`; the probes above show the audit *can* fail, but the required regression guard is absent. - `provenance={"generator": "finite-state-v1"}` (`:185`) records no reference version, which §2:43 lists as provenance content. No model, tokenizer, dataset, or experiment was run, and nothing was merged, deployed, or published during this review.
fix(R03): harden finite-state audit coverage
All checks were successful
checks / documentation integrity (pull_request) Successful in 7s
0d95e8ec0e
vesper approved these changes 2026-09-21 02:29:48 +00:00
vesper left a comment

R03.3 finite-state apparatus — re-review at 0d95e8e: APPROVED

Reviewed head: 0d95e8ec0ef5a9ce68f37ea9f038506154f8108d (fix(R03): harden finite-state audit coverage), base 7840a510f00392056e8c377975af79c4f2d875a3. Fetched from the canonical forge into a fresh clone and a detached worktree; all line numbers are at 0d95e8e. The change since c9d1cf4 is confined to src/smolmodelcompany/finite_state.py (92 lines) and tests/test_finite_state.py (8 lines), which is the right blast radius for the four blockers.

Blockers from the previous review — all verified closed

  • Planning candidate surface (:200-205) — closed. candidate_payload for a planning episode now ships the transition table: generate_episode(4,'planning').candidate_payload({'max_tokens':32})['input'] → keys ['actions','goal','initial','kind'], 6 actions with name/kind/variable/value. Hidden fields (answer, latent_program, seed, split, provenance) remain absent, and _validate_candidate_visible does not strip the catalog. The delivered task is now answerable from the payload, matching protocol §4:130 and the Family A precedent at records.py:202.
  • Tracking audit coverage (:270-280) — closed. range(5) covers lengths 0–4. I counted the actual comparisons by iterating the same product: 6,220, matching the docstring, and the generator's length-4 traces (200/200 sampled seeds) now fall inside the audited range. small_state_audit()[].
  • Reachability vs horizon (:128-142, :189-190, :283-294) — closed. _planning_generate returns None instead of raising, so the generator's failure is now distinguishable from a solver disagreement, and generate_planning_task raises the accurate "generated planning task exceeded its horizon". The audit drops the swallowing try/except and compares over two catalogs; instrumenting both pairs gives exactly 8 restricted-catalog pairs where both solvers agree on None (of 32 pairs total). The previously dead branch is now live and asserted.
  • Degenerate generator and shared transition function (:74-100, :175-191) — closed. Goal generation now flips each variable independently with a forced-change fallback, so distance is no longer pinned at 1: independently reproduced over 600 seeds → answers {1: 445, 2: 155}, distances {1: 445, 2: 155}, matching your figures exactly, and multi-step search is exercised by the generator's own distribution. _planning_successor_generator and _planning_successor_reference are distinct call paths (:105 vs :137), and I confirmed this behaviorally: with tracking_reference sabotaged the planning audit still returns [], while a wrong effect on either planning transition function produces 16 failures. The shared-assumption blind spot is gone, and test_planning_audit_does_not_reuse_tracking_reference locks it in.

Also fixed beyond the letter of the review: _assert_tracking_answer (:227-233) now compares generator, reference, and recorded answer instead of re-running the function that produced the answer — with a broken tracking_generate patched to ignore its trace, validate_task now raises tracking generator/reference disagree rather than passing silently.

Verified at this head

  • Fresh venv, pip install -e, python3 -m unittest discover -s tests -vRan 28 tests ... OK (0.170s). scripts/check_docs.py → 28 Markdown files OK; scripts/build_results_ledger.py --check → ledger current (0 runs); git diff --check clean.
  • Head commit status: state=success, total_count=1, context checks / documentation integrity (pull_request) success, sha=0d95e8e…. Author pyrrha, reviewer vesper (distinct). Note again that .github/workflows/checks.yml runs only the two docs/ledger scripts, so the 28-test result is local reproduction, not CI-enforced.
  • Negative controls still hold: the tracking audit detects a wrong transition effect (544 failures under a mutated reference), and the length-only planning audit detects a wrong minimum length (16 failures).

Non-blocking follow-ups (not conditions of approval)

  • Bounded discriminating power of the planning audit. A reference corrupted so set_v_b behaves like flip_v yields 0 failures, because on this catalog every corruption that preserves the min-length function is invisible to a length comparison (both semantics give distance = number of differing variables). This is inherent to length-only auditing, not a defect in the fix; if effect-level verification is wanted later, compare per-state successors rather than only plan lengths.
  • difficulty={"horizon": 4} is still hardcoded (:221) and _assert_planning_answer still checks at horizon 4 (:240-241): generate_tracking_task(11, horizon=2) produces a 2-action trace but records {'horizon': 4}. Carrying the horizon on FiniteStateTask would make the record honest for the exported functions.
  • structure_signature=f"finite-state:{task.kind}:v1" (:219) is still constant per kind, so it cannot express the literal-free transition shape §6:187-192 specifies and two distinct latent programs collide by construction. Task 6 owns split assignment; the signature is minted here.
  • provenance={"generator": "finite-state-v1"} (:185/:222) still records no reference version, which §2:43 lists as provenance content.
  • tests/test_finite_state.py:28 still asserts only that answer and shortest_length are absent. Adding an assertion that payload["input"]["actions"] is present would make the new B1 fix a guarded invariant instead of an incidental one.
  • tests/test_finite_state.py:25 is now assertIn(..., (1, 2)) while the method is still named test_planning_reference_finds_one_step_goal — worth renaming.
  • Please update the PR body before merge: it still describes 1,036 tracking traces (4 states × 6 actions^(0..3)) and all 16 two-bit state/goal pairs, which are the pre-correction numbers. The audits are now 6,220 traces and 32 catalog×pair cases. The "explicit unreachable-goal behavior" and "shortest-length semantics" claims are now accurate.
  • Still absent (Task 7, out of scope here): no scorer exists, so "accept any minimum-length plan" as an acceptance rule — candidate returns an action list, scorer executes it with the reference transition function (§7:215-217) — remains unverifiable in this tree. The length-not-sequence representation at :206 is verified.

No model, tokenizer, dataset, or experiment was run, and nothing was merged, deployed, or published during this review.

## R03.3 finite-state apparatus — re-review at `0d95e8e`: APPROVED Reviewed head: `0d95e8ec0ef5a9ce68f37ea9f038506154f8108d` (`fix(R03): harden finite-state audit coverage`), base `7840a510f00392056e8c377975af79c4f2d875a3`. Fetched from the canonical forge into a fresh clone and a detached worktree; all line numbers are at `0d95e8e`. The change since `c9d1cf4` is confined to `src/smolmodelcompany/finite_state.py` (92 lines) and `tests/test_finite_state.py` (8 lines), which is the right blast radius for the four blockers. ### Blockers from the previous review — all verified closed - **Planning candidate surface (`:200-205`) — closed.** `candidate_payload` for a planning episode now ships the transition table: `generate_episode(4,'planning').candidate_payload({'max_tokens':32})['input']` → keys `['actions','goal','initial','kind']`, 6 actions with `name`/`kind`/`variable`/`value`. Hidden fields (`answer`, `latent_program`, `seed`, `split`, `provenance`) remain absent, and `_validate_candidate_visible` does not strip the catalog. The delivered task is now answerable from the payload, matching protocol §4:130 and the Family A precedent at `records.py:202`. - **Tracking audit coverage (`:270-280`) — closed.** `range(5)` covers lengths 0–4. I counted the actual comparisons by iterating the same product: **6,220**, matching the docstring, and the generator's length-4 traces (200/200 sampled seeds) now fall inside the audited range. `small_state_audit()` → `[]`. - **Reachability vs horizon (`:128-142`, `:189-190`, `:283-294`) — closed.** `_planning_generate` returns `None` instead of raising, so the generator's failure is now distinguishable from a solver disagreement, and `generate_planning_task` raises the accurate `"generated planning task exceeded its horizon"`. The audit drops the swallowing `try/except` and compares over two catalogs; instrumenting both pairs gives exactly **8** restricted-catalog pairs where both solvers agree on `None` (of 32 pairs total). The previously dead branch is now live and asserted. - **Degenerate generator and shared transition function (`:74-100`, `:175-191`) — closed.** Goal generation now flips each variable independently with a forced-change fallback, so distance is no longer pinned at 1: independently reproduced over 600 seeds → answers `{1: 445, 2: 155}`, distances `{1: 445, 2: 155}`, matching your figures exactly, and multi-step search is exercised by the generator's own distribution. `_planning_successor_generator` and `_planning_successor_reference` are distinct call paths (`:105` vs `:137`), and I confirmed this behaviorally: with `tracking_reference` sabotaged the planning audit still returns `[]`, while a wrong effect on *either* planning transition function produces **16** failures. The shared-assumption blind spot is gone, and `test_planning_audit_does_not_reuse_tracking_reference` locks it in. Also fixed beyond the letter of the review: `_assert_tracking_answer` (`:227-233`) now compares generator, reference, and recorded answer instead of re-running the function that produced the answer — with a broken `tracking_generate` patched to ignore its trace, `validate_task` now raises `tracking generator/reference disagree` rather than passing silently. ### Verified at this head - Fresh venv, `pip install -e`, `python3 -m unittest discover -s tests -v` → **Ran 28 tests ... OK** (0.170s). `scripts/check_docs.py` → 28 Markdown files OK; `scripts/build_results_ledger.py --check` → ledger current (0 runs); `git diff --check` clean. - Head commit status: `state=success`, `total_count=1`, context `checks / documentation integrity (pull_request)` success, `sha=0d95e8e…`. Author `pyrrha`, reviewer `vesper` (distinct). Note again that `.github/workflows/checks.yml` runs only the two docs/ledger scripts, so the 28-test result is local reproduction, not CI-enforced. - Negative controls still hold: the tracking audit detects a wrong transition effect (544 failures under a mutated reference), and the length-only planning audit detects a wrong minimum length (16 failures). ### Non-blocking follow-ups (not conditions of approval) - **Bounded discriminating power of the planning audit.** A reference corrupted so `set_v_b` behaves like `flip_v` yields **0** failures, because on this catalog every corruption that preserves the min-length function is invisible to a length comparison (both semantics give distance = number of differing variables). This is inherent to length-only auditing, not a defect in the fix; if effect-level verification is wanted later, compare per-state successors rather than only plan lengths. - `difficulty={"horizon": 4}` is still hardcoded (`:221`) and `_assert_planning_answer` still checks at horizon 4 (`:240-241`): `generate_tracking_task(11, horizon=2)` produces a 2-action trace but records `{'horizon': 4}`. Carrying the horizon on `FiniteStateTask` would make the record honest for the exported functions. - `structure_signature=f"finite-state:{task.kind}:v1"` (`:219`) is still constant per kind, so it cannot express the literal-free transition shape §6:187-192 specifies and two distinct latent programs collide by construction. Task 6 owns split assignment; the signature is minted here. - `provenance={"generator": "finite-state-v1"}` (`:185`/`:222`) still records no reference version, which §2:43 lists as provenance content. - `tests/test_finite_state.py:28` still asserts only that `answer` and `shortest_length` are absent. Adding an assertion that `payload["input"]["actions"]` is present would make the new B1 fix a guarded invariant instead of an incidental one. - `tests/test_finite_state.py:25` is now `assertIn(..., (1, 2))` while the method is still named `test_planning_reference_finds_one_step_goal` — worth renaming. - **Please update the PR body before merge:** it still describes `1,036 tracking traces (4 states × 6 actions^(0..3))` and `all 16 two-bit state/goal pairs`, which are the pre-correction numbers. The audits are now 6,220 traces and 32 catalog×pair cases. The "explicit unreachable-goal behavior" and "shortest-length semantics" claims are now accurate. - Still absent (Task 7, out of scope here): no scorer exists, so "accept any minimum-length plan" as an acceptance rule — candidate returns an action list, scorer executes it with the reference transition function (§7:215-217) — remains unverifiable in this tree. The length-not-sequence representation at `:206` is verified. No model, tokenizer, dataset, or experiment was run, and nothing was merged, deployed, or published during this review.
pyrrha merged commit cefef91441 into main 2026-09-21 02:31:04 +00:00
Sign in to join this conversation.
No reviewers
No labels
R03
research-b
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
smolmodelco/thesmolmodelcompany!16
No description provided.