SLM-312: seedward and on-policy state sources with matched mixtures - #848
SLM-312: seedward and on-policy state sources with matched mixtures#848Tyler-R-Kendrick wants to merge 7 commits into
Conversation
- coverage_class per frozen row (OBSERVABLE_PROMPT / OBSERVABLE_REQUEST_ONLY / UNKNOWN); all 51 rows are OBSERVABLE_REQUEST_ONLY pre-inventory. - Matched requests differ only by the production ensure_prompt_inventory suffix; request-derived, no hidden gold channel. - X22 deterministic arm: paired v2-strict delta +0.3125 (5/16 vs 0/16, Wilson [0.142, 0.556]) exceeds the predeclared 0.10 minimum; no regressions on observable rows. - AR tiny baseline: 6/32 decodes completed (all v2-fail partials), 26 decode_timeout (600-step checkpoint constrained-forest cost per SLM-294 evidence); reported unmeasured, not evidence. Rico rows not_run with SLM-294 cost evidence. - 100-record blind slot-observability audit: raw agreement 97%, kappa 0.0 (skew caveat); finding: v2 coverage detector misses inline slot enumeration -> false prompt_contract_unknown (append-only; no metric change). - 8 harness tests; new component harness.experiments.slm301_prompt_observability v1.
…ints - decode_outcome.py taxonomy with strict precedence (fallback never counts as model success); per-request budget/elapsed/forwards/ verifier/fallback/stop-reason records. - eval_runner additive integration: details[] gain decode_outcome, stop_reason, fallback_used; suite metrics gain decode_outcome_counts. - Census: only slm230 committed-SHA verifiable; hash-pinned the two remediated nonzero-timeout checkpoints; scoreboard classes separated (runtime_timeout/fallback/unmeasured/model_behavior). - Preregistered 1x vs 10x budget sweep: all 12 cells not_rerunnable (v0 checkpoints fail current output-contract check); the 10x-flip question is honestly UNANSWERED by re-decode. Recommendation: retrain the remediated recipe on contract v2, then re-eval at both budgets. - 25 new tests; harness.model_build.eval v50; component harness.experiments.slm303_decode_budget_audit v1; gates.ship v3 threshold mirror fixed.
- slm308_distance_oracle.py: bounded reverse-BFS over canonical AST fingerprints via the real extended 11-action transitions; EXACT / BOUNDED / UNKNOWN labels (budget never conflated with distance); cache keyed by action-schema version, grammar sha, inventory, target and state hashes. - tree_edit_diffusion: value_label_mode (bounded_distance default for new configs; pre-field checkpoints inject mutation_count for behavior parity, format stays 2); normalized oracle cost-to-go with UNKNOWN masked out of MSE; pairwise progress margin loss (independently tested); decode paths never read gold distance (audit test). - Matched fixture experiment (mutation_count vs bounded_distance, identical budgets, near-gold + seed-trajectory states): rank corr 0.433->0.505 (+0.072 < 0.10), beam regret 0.889->0.889 (+0.0 < 0.05), Brier 0.031->0.004, UNKNOWN coverage 0.175. Preregistered thresholds written before results; verdict honestly rejected at this budget. - 16 new tests; slm308 component v1; slm299 component v3.
- Proposal instrumentation: per enumerated candidate — action, factor score, applicability, rejection reason (23 machine-readable codes), budget consumption; deterministic order; additive evidence keys. - Distribution audit: training-target vs decode-demand action distributions by source/suite; dead-candidate rate ~0.98, applicable-ADD recall 0.10 baseline; preregistered reweighting rule. - corruption_action_distribution knob (gold corpus untouched; default off = historical uniform; parity tested) and stop_slot_accounting legacy|corrected arm (STOP consumes an expansion slot only when its frozen candidate is retained; deterministic, regression-tested). - Matched 2x2 (ADD-balanced x STOP arms, isolated levers): ADD target share 0.177 -> 0.328 (T1 +0.150 >= 0.10 both STOP arms), recall 0.10 -> 0.16, corrected STOP budget <= legacy; verdicts adopted per preregistered rules; loss reweighting deferred to preserve isolation. - 10 new tests; slm299 component v4; slm310 component v1.
- State-source harness: gold_only (existing corruption chain), seedward
(offline oracle-guided walk from the seed toward gold, strictly
distance-decreasing valid intermediates), on_policy (immutable
content-addressed beam-trajectory snapshots: wrong states, verifier
failures, abstentions); explicit provenance + gold-visibility policy
per row; sha256 rows + tamper-evident manifest; fail-closed leakage
guards (train vs held-out AST fingerprints).
- Matched {gold_only, seedward, on_policy, mixed} arms with predeclared
weights/caps, identical model/steps/optimizer/seeds; evaluated on
held-out seed trajectories. Verdict: rejected per preregistered
primary gate (beam regret improvement 0.0 < 0.05); secondary signal
(value rank corr -0.258 -> +0.258 for non-gold arms) recorded as
wiring evidence only.
- 9 new tests; slm312 component v1.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR expands the tree-edit action space, adds bounded-distance and state-source experiments, introduces decode and LangSmith telemetry, supports checkpoint migration, and adds reports, documentation, registry entries, and tests for the new behavior. ChangesTelemetry and evaluation reporting
Extended tree-edit language and experiments
State-source acquisition experiment
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (24)
scripts/run_agentv_eval.mjs-58-58 (1)
58-58: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win500ms timeout is aggressive for an external API call.
AbortSignal.timeout(500)gives very little headroom for a real network round-trip toapi.smith.langchain.com, especially right afterevaluate()has already run. The Python sibling flushes with a 2.0s timeout (pertests/test_runtime_trace.py:("flush", (), {"timeout": 2.0})). At 500ms, this export is likely to fail (silently, via the catch) under normal latency, undermining the intended telemetry.⏱️ Suggested fix
- signal: AbortSignal.timeout(500), + signal: AbortSignal.timeout(2000),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/run_agentv_eval.mjs` at line 58, Increase the AbortSignal.timeout value in the export request to provide approximately 2 seconds for the external API round trip, matching the Python sibling’s flush timeout and preserving the existing request and catch behavior.docs/design/iter-slm312-state-sources-20260724.json-458-467 (1)
458-467: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
code_dirty: truemakes this evidence non-reproducible fromcode_commit. The stamp pinsd8f35563…but the working tree had uncommitted changes, so nobody can regenerate this payload from that commit. Consider regenerating the artifact from a clean tree before merge (or noting the deviation append-only) so the durable JSON evidence is reproducible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/iter-slm312-state-sources-20260724.json` around lines 458 - 467, Regenerate the artifact represented by the version_stamp from a clean working tree so code_dirty is false and the payload is reproducible from code_commit d8f35563d231e3b404b0fb0e213d68aee1b00a9d; if the uncommitted changes must remain, document that deviation append-only in the artifact instead.Source: Coding guidelines
scripts/run_slm312_state_sources.py-452-465 (1)
452-465: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
render_markdowndrops the honesty context the JSON payload carries. The generated summary reports the gate metric and the leak-guard claim without their qualifiers, so the Markdown reads stronger than the payload it is rendered from.
scripts/run_slm312_state_sources.py#L452-L465: render the beam-regret column with itsbeam_regret_n/beam_regret_excludedcounts, and add a line forsnapshot.manifest.config.dropped_leak_collisions(count and reason) next to the coverage table.docs/design/iter-slm312-state-sources-20260724.md#L15-L22: regenerate this artifact so the headline table and Honesty section show the regret sample size and the one dropped held-out fingerprint collision.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/run_slm312_state_sources.py` around lines 452 - 465, The render_markdown output omits honesty qualifiers from the JSON payload. In scripts/run_slm312_state_sources.py lines 452-465, update render_markdown to include beam_regret_n and beam_regret_excluded in the beam-regret table column, and add snapshot.manifest.config.dropped_leak_collisions with its count and reason beside the coverage table. Regenerate docs/design/iter-slm312-state-sources-20260724.md lines 15-22 so its headline table and Honesty section reflect the regret sample size and dropped held-out fingerprint collision.src/slm_training/harnesses/experiments/slm312_state_sources.py-747-748 (1)
747-748: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winFingerprint dedupe is global across records, so identical states get dropped for later records.
seenlives outside the per-record loop, but rows are labeled relative to that record's gold target and inventory — the same program text carries a differentdistance_label,supervised_edit, andchild_sourceper record. A shared shallow state (very common early in seed-originated beams) therefore counts toward one record only and silently reduces later records' contribution undermax_states_per_record. The fixture run happens to hit 18 rows = 6 records × cap 3, so it's latent today.Scope the dedupe per record; cross-record duplicates are still accounted for by
source_coverageand deduped again inbuild_arm_rows.🐛 Proposed fix
rows: list[StateSourceRow] = [] - seen: set[str] = set() rollout_config = {for record in records: gold = parse_statements(record.openui or "") if gold is None: continue + seen: set[str] = set()Also applies to: 796-803
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/slm_training/harnesses/experiments/slm312_state_sources.py` around lines 747 - 748, Move the seen fingerprint set from the outer scope into the per-record loop so deduplication is reset for each record. Update the loop around rows and the record-processing logic to use a fresh set per record, while preserving cross-record accounting through source_coverage and final deduplication in build_arm_rows.scripts/run_slm312_state_sources.py-513-515 (1)
513-515: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
--md-outparent directory is never created. Onlyjson_out.parentis created, so a custom--md-outin a new directory raisesFileNotFoundErrorafter the full sweep has already been paid for.🐛 Proposed fix
args.json_out.parent.mkdir(parents=True, exist_ok=True) args.json_out.write_text(json.dumps(payload, indent=2) + "\n", encoding="utf-8") + args.md_out.parent.mkdir(parents=True, exist_ok=True) args.md_out.write_text(render_markdown(payload), encoding="utf-8")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/run_slm312_state_sources.py` around lines 513 - 515, Update the output-writing flow around args.json_out and args.md_out so the parent directory for args.md_out is also created with parents enabled before render_markdown(payload) writes the file. Preserve the existing JSON directory creation and output behavior.src/slm_training/harnesses/experiments/slm312_state_sources.py-920-937 (1)
920-937: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winBudget truncation can silently violate the predeclared mixture weights.
quota = max(1, round(weight * per_arm_budget))can sum aboveper_arm_budget(e.g. two 0.5 weights with budget 3 → 2 + 2 = 4). Because sources are appended insorted(weights)order and the finalselected[:per_arm_budget]truncates from the tail, the overflow is taken entirely from the alphabetically-last source — somixedwould drift from its preregistered weights without any record of it. The fixture (8/8/8 = 24) doesn't hit it, but the arm-matching claim depends on it.Either allocate with largest-remainder so quotas sum to the budget, or shuffle
selectedwith the arm rng before truncating so the loss is proportional.🐛 Proposed fix (proportional truncation)
n = min(quota, len(unique)) selected.extend(rng.sample(unique, n) if n < len(unique) else unique) - return selected[:per_arm_budget] + if len(selected) > per_arm_budget: + # Drop proportionally across sources instead of truncating the + # alphabetically-last source out of the arm. + selected = rng.sample(selected, per_arm_budget) + return selected🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/slm_training/harnesses/experiments/slm312_state_sources.py` around lines 920 - 937, Update the source-selection logic around the quota loop to prevent deterministic truncation from favoring the alphabetically last source. After collecting the per-source selections, use the arm RNG to shuffle the combined selected rows before applying the per_arm_budget slice, preserving proportional loss when rounded quotas exceed the budget.src/slm_training/harnesses/experiments/slm312_state_sources.py-544-578 (1)
544-578: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win"Strictly improving" isn't guaranteed when the parent label is UNKNOWN.
When
d_current is Nonethe guard on Line 572 is skipped, so any oracle-measurable child is accepted — no strict decrease is proven. That case is real, not theoretical: the test at Lines 247-252 oftests/test_harnesses/experiments/test_slm312_state_sources.pynotes the bare seed is UNKNOWN at these budgets, which is exactly the parentacquire_seedwardstarts from. As a result the claim inacquire_seedward(Lines 666-669) and in the module docstring (Lines 10-13) that every seedward step is "strictly distance-decreasing by construction" overstates what the oracle certified for the first step.Suggest qualifying the docstrings (or adding a
ponytail:comment naming the limitation and its upgrade path, e.g. deeper budgets for the parent label), so the artifact's honesty claims match the enforced invariant.📝 Suggested docstring tightening
- """Strictly-improving child with the smallest oracle distance, or None. + """Child with the smallest oracle distance, or None. + + Strictly improving whenever the PARENT is oracle-measurable. When the + parent label is UNKNOWN (shallow budgets), any measurable child is + accepted — ponytail: no strict decrease is certified for that step; + upgrade path is a deeper oracle budget for the parent label.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/slm_training/harnesses/experiments/slm312_state_sources.py` around lines 544 - 578, Qualify the strict-improvement claims in the module docstring and acquire_seedward documentation to state that decrease is guaranteed only when the parent has a known oracle distance; when d_current is None, the first measurable child is accepted without a proven decrease. Preserve the existing selection logic in the shown function, and mention the limitation or future path of using deeper oracle budgets to establish the parent label.Source: Coding guidelines
src/slm_training/harnesses/experiments/slm312_state_sources.py-616-618 (1)
616-618: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
k_maxabove 3 is silently clamped by the hardcoded tuple.The parameter advertises an arbitrary chain depth but iteration is fixed to
(1, 2, 3), sok_max=5yields only 3 chains with no signal. Either derive the loop fromk_maxor document the hard ceiling.🐛 Proposed fix
- for k in (1, 2, 3): - if k > k_max: - break + for k in range(1, k_max + 1):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/slm_training/harnesses/experiments/slm312_state_sources.py` around lines 616 - 618, Update the chain-depth iteration in the surrounding experiment harness to honor arbitrary k_max values by iterating through the range from 1 to k_max rather than the hardcoded (1, 2, 3) tuple; preserve the existing early-stop behavior for lower limits.tests/test_harnesses/experiments/test_slm312_state_sources.py-131-141 (1)
131-141: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
assert "final" in kindsis order-dependent and can flake.
acquire_on_policysorts candidates bypriority = {"verifier_failure_cone": 0, "final": 1, "visited": 2}and then stops atmax_states_per_record=3. With an untrained model atbeam_width=3over up to 6 search steps, three or more distinct failure-cone states are easy to produce, which would evictfinalbefore it is ever labeled. Raise the cap here (or assert on the un-capped candidate set) so the test isn't sensitive to how many proposals the random-init policy happens to fail.💚 Proposed fix
- max_states_per_record=3, + max_states_per_record=8,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_harnesses/experiments/test_slm312_state_sources.py` around lines 131 - 141, Update the test around acquire_on_policy to avoid asserting final survives the capped selection: increase max_states_per_record sufficiently to retain the final candidate under failure-cone-heavy random policies, or validate kinds from the uncapped candidate set instead. Preserve the existing nonempty and allowed-kind assertions while removing sensitivity to candidate ordering and cap pressure.scripts/run_slm312_state_sources.py-197-206 (1)
197-206: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard the empty-arm case, and soften the "identical batch order" claim.
Two things in this loop:
order.randrange(len(arm_rows))raisesValueError: empty rangewhen an arm's pool is empty. That's reachable:build_arm_rowsyields nothing for a source whose acquisition produced no rows (e.g.acquire_seedwardfinds no oracle-improving child for any record), which would abort the run mid-sweep instead of recording an honest empty arm.- The comment on Line 198 (and the module docstring, Lines 19-21) says arms share batch order, but the rng only produces a shared index stream — arm pools differ in size (17 / 8 / 18 / 24 in the recorded run), so the sampled rows and effective epoch coverage differ by arm.
🐛 Proposed fix
+ if not arm_rows: + raise ValueError("arm has no rows; refusing to train an empty arm") model = TreeEditDiffusionModel.from_records(records, config=config, device="cpu") optimizer = torch.optim.Adam(model.trainable_parameters(), lr=3e-3) - order = random.Random(0) # identical batch order across arms + # Shared rng index stream across arms; pool sizes differ, so the drawn + # rows differ by arm (matched budget, not matched batch contents). + order = random.Random(0)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/run_slm312_state_sources.py` around lines 197 - 206, Guard the training loop around the arm pool so empty arm_rows are recorded as an honest empty arm without calling order.randrange or running optimization. Update the nearby “identical batch order” comment and corresponding module docstring wording to describe only a shared RNG/index stream, since differing pool sizes produce different sampled rows and coverage; preserve normal training for non-empty arms.docs/design/iter-slm303-decode-budget-audit-20260724.md-10-11 (1)
10-11: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCensus totals do not reconcile.
verified 1 + mismatch 0 + unverifiable 305 = 306, against216local checkpoints enumerated (roster rows115). No combination of the stated populations yields 306, so either the verification counters are tallied over a different population than the enumeration, or one of them double-counts. Please reconcile — and state the denominator explicitly in the rendered line if the two axes genuinely differ.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/iter-slm303-decode-budget-audit-20260724.md` around lines 10 - 11, Reconcile the checkpoint census totals in the document so the verification counters align with the enumerated population, or explicitly identify the distinct denominator for each axis. Update the rendered summary line to state each denominator and ensure the reported verified, mismatch, and unverifiable counts are internally consistent.docs/design/langsmith-telemetry-smoke-20260724.json-41-50 (1)
41-50: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUpdate the language-telemetry smoke stamp to the post-bump registry versions
versions.jsonnow points both component entries tov54andv4; this JSON’sharness.model_build.eval: v52andevals.agentv: v3stamps do not match the current registry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/langsmith-telemetry-smoke-20260724.json` around lines 41 - 50, Update the version_stamp.components entries in the language-telemetry smoke stamp to match the registry: set harness.model_build.eval to v54 and evals.agentv to v4. Leave the remaining version stamp fields unchanged.Source: Coding guidelines
src/slm_training/runtime/telemetry/trace.py-110-117 (1)
110-117: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winKeep trace attributes to metrics-only values.
extra.metadataforwards**self.trace.attributesdirectly, and callers can already pass attributes such astrain-dirpaths. Since the export contract is limited to run/suite metrics, version stamps, gate verdicts, and AgentV summaries, include or derive only metadata in that scope rather than arbitrary caller-provided data.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/slm_training/runtime/telemetry/trace.py` around lines 110 - 117, Update the telemetry export block surrounding the trace metadata construction to stop forwarding all values from self.trace.attributes. Build extra.metadata only from the approved run/suite metrics, version stamps, gate verdicts, and AgentV summary fields, while retaining the w3c_trace_id and service metadata.docs/design/iter-slm303-decode-budget-audit-20260724.md-42-48 (1)
42-48: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winCensus classification is still self-referential for the audit output.
_disposition()currently keys annotations fromcensus["scoreboards"], but the same script also emitsdocs/design/iter-slm303-decode-budget-audit-20260724.json, so that JSON appears in the disposition as an annotated artifact. That makes the append-only ledger point at its own generated payload for the audit’s own row-level verdicts; keep the audit JSON out ofcensus["scoreboards"], then regenerate the JSON and Markdown.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/iter-slm303-decode-budget-audit-20260724.md` around lines 42 - 48, Update the census-building logic used by _disposition() to exclude docs/design/iter-slm303-decode-budget-audit-20260724.json from census["scoreboards"], preventing the audit output from annotating itself. Then regenerate the audit JSON and Markdown artifacts so the append-only ledger contains only external scoreboard entries.scripts/run_slm303_decode_budget_audit.py-706-706 (1)
706-706: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAn unpaired cell is misreported as "blocked by the output-contract gate."
pair_cellsemits{"status": "unpaired", ...}dicts with nooutcome_baselinekey, sop.get("outcome_baseline") is Noneis True for them and they land innot_rerunnable. The verdict at Lines 733-738 then asserts those cells were blocked by the output-contract gate with "no migration path" — but a simply-missingbudget10xcell (e.g. one sweep command not yet run) is a different fact entirely, and this is the audit's headline disposition.🐛 Suggested fix
- not_rerunnable = [p for p in pairs if p.get("outcome_baseline") is None] + not_rerunnable = [ + p + for p in pairs + if p.get("status") == "paired" and p.get("outcome_baseline") is None + ] + unpaired = [p for p in pairs if p.get("status") != "paired"]and surface
unpairedseparately incounts/sweep_verdictinstead of folding it into the contract-gate narrative.Also applies to: 730-738
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/run_slm303_decode_budget_audit.py` at line 706, Separate entries with status "unpaired" from the not_rerunnable classification in the audit flow around pair_cells and the verdict logic near counts/sweep_verdict. Track unpaired cells in their own count and surface them as a distinct sweep_verdict disposition, while retaining only genuinely contract-gated entries in not_rerunnable and the “no migration path” narrative.src/slm_training/resources/versions.json-6301-6301 (1)
6301-6301: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPrefer appending over rewording a landed history entry.
This rewrites the prose of the existing
v223entry (dated 2026-07-22) instead of appending. As per coding guidelines the registry history is "newest-first append-only"; if the original note needs clarification, add a same-versionno-bump:entry above it and leave the landed text intact.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/slm_training/resources/versions.json` at line 6301, The existing v223 registry note must remain unchanged because the history is append-only. Restore its landed text, then add a same-version “no-bump:” entry above it containing the clarification, preserving newest-first ordering.Source: Coding guidelines
src/slm_training/evals/agentv.py-189-198 (1)
189-198: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMalformed
trace.jsoncan abort AgentV publication.The
except (OSError, json.JSONDecodeError)guard misses two reachable cases: a valid-JSON non-object ([],"x",null) makes.getraiseAttributeError, and a non-UTF-8 file raisesUnicodeDecodeError(aValueError, not anOSError). Either propagates out ofpublish_agentv_evaluationand fails the whole publication over optional trace metadata.🛡️ Suggested fix
try: - trace_id = str(json.loads(path.read_text(encoding="utf-8")).get("trace_id", "")) - except (OSError, json.JSONDecodeError): + payload = json.loads(path.read_text(encoding="utf-8")) + except (OSError, ValueError): # unreadable, non-UTF-8, or invalid JSON return None + if not isinstance(payload, dict): + return None + trace_id = str(payload.get("trace_id", "")) return trace_id if re.fullmatch(r"[0-9a-f]{32}", trace_id) else None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/slm_training/evals/agentv.py` around lines 189 - 198, Update _run_trace_id to treat all malformed or unreadable trace.json contents as absent optional metadata: guard JSON results before calling .get and handle UnicodeDecodeError alongside the existing read/parse failures. Preserve returning only a valid 32-character lowercase hexadecimal trace ID, while returning None for non-object JSON, invalid encoding, or other malformed content.scripts/run_slm303_decode_budget_audit.py-874-877 (1)
874-877: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
--json-out/--md-outinto a new directory raisesFileNotFoundError.
run_census(Line 425) andrun_sweep_cell(Line 616) bothmkdir(parents=True, exist_ok=True)before writing; this writer does not, so the documented CLI overrides fail on any path whose parent does not already exist.🛡️ Suggested fix
payload["version_stamp"] = build_version_stamp(COMPONENT, EVAL_COMPONENT) + json_out.parent.mkdir(parents=True, exist_ok=True) + md_out.parent.mkdir(parents=True, exist_ok=True) json_out.write_text( json.dumps(payload, indent=2, default=str) + "\n", encoding="utf-8" )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/run_slm303_decode_budget_audit.py` around lines 874 - 877, Update the output-writing flow around json_out.write_text and md_out.write_text to create both parent directories before writing, using mkdir(parents=True, exist_ok=True) as in run_census and run_sweep_cell. Preserve the existing JSON and Markdown serialization behavior.src/slm_training/harnesses/model_build/decode_outcome.py-52-78 (1)
52-78: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
erroris accepted but never consulted.Every caller passes it (
eval_runner.pyLine 1087,scripts/run_slm303_decode_budget_audit.pyLines 157 and 548), yet it cannot affect the verdict:parse_ok=Nonewitherror="…"still returnsmodel_valid. For a taxonomy whose stated point is to stop laundering failures into model successes, an unread failure signal is a trap. Either treat it as contrary evidence or remove it from the signature.🐛 Suggested fix
if abstained: return MODEL_ABSTAIN - # parse_ok None = parse not evaluated; a produced candidate with no - # contrary evidence is a model output, not a failure verdict. - return MODEL_INVALID if parse_ok is False else MODEL_VALID + # parse_ok None = parse not evaluated; a produced candidate with no + # contrary evidence is a model output, not a failure verdict. A recorded + # error IS contrary evidence, even when no parse verdict was computed. + if parse_ok is False or (parse_ok is None and error): + return MODEL_INVALID + return MODEL_VALID🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/slm_training/harnesses/model_build/decode_outcome.py` around lines 52 - 78, Update classify_decode_outcome so the error parameter cannot be silently ignored: treat a non-null error as contrary evidence when determining the final model verdict, while preserving the existing precedence for harness exceptions, timeouts, fallback outputs, and abstentions. Ensure parse_ok=None with an error no longer returns MODEL_VALID, or remove error from the function signature and all callers if it is not intended to affect classification.src/slm_training/models/tree_edit_diffusion.py-1190-1210 (1)
1190-1210: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate declared weights against realizable inverse actions, not just
ACTION_IDS.
STOPandINSERT_STATEMENTare validACTION_IDSkeys but have no entry inINVERSE_TO_MUTATION_KINDS, so weight placed on them is silently unrealizable:sample_mutationdraws them, finds no realizing mutation kind, and burns a retry. A degenerate config such as{"STOP": 1.0}exhausts all 12 retries for every record, so every record is counted asskippedandtraining_lossreturns a zero tensor — a training run that no-ops instead of failing closed.🛡️ Fail closed on unrealizable inverse actions
weights: dict[int, float] = {} for name, weight in declared.items(): if name not in ACTION_IDS: raise ValueError( f"corruption_action_distribution names unknown action {name!r} " f"(known: {sorted(ACTION_IDS)})" ) + if weight > 0 and ACTION_IDS[name] not in INVERSE_TO_MUTATION_KINDS: + raise ValueError( + f"corruption_action_distribution weights {name!r}, which no " + "mutation kind can realize as an inverse action" + ) if weight < 0:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/slm_training/models/tree_edit_diffusion.py` around lines 1190 - 1210, Update _inverse_action_weights to validate declared action names against the realizable inverse-action keys in INVERSE_TO_MUTATION_KINDS, rejecting STOP, INSERT_STATEMENT, or any other action without a corresponding mutation kind before returning weights. Preserve the existing unknown-name, negative-weight, and all-zero validations while ensuring a configuration with only unrealizable actions fails closed instead of reaching sample_mutation.scripts/run_slm299_reachability_audit.py-391-401 (1)
391-401: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
--compare --mode v1yields a v1-vs-v1 self-comparison labelled "Old (v1) vs extended".
build_reportalways computes the comparison arm withmode="v1", so pairing--comparewith--mode v1produces identical suites, zero flips, and a report section claiming an extended-space comparison. Reject the combination (or forcemode="extended"when comparing) so the published evidence cannot be mislabelled.🛡️ Guard the flag combination
args = parser.parse_args(argv) + if args.compare and args.mode != "extended": + parser.error("--compare compares the v1 space against extended; use --mode extended") generated_at = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/run_slm299_reachability_audit.py` around lines 391 - 401, Reject the invalid --compare --mode v1 combination in the argument-validation or audit entry flow around the mode and compare parser options, before build_report runs. Allow --compare only with mode="extended" so the existing v1 comparison arm and its “Old (v1) vs extended” report remain correctly labelled.tests/test_harnesses/experiments/test_slm310_action_alignment.py-151-157 (1)
151-157: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale comment: the rank-ordering invariant is described but never asserted.
"Deterministic order: ranks are strictly increasing per state" sits above the applicability check and nothing verifies it. Either assert it or drop the comment.
💚 Assert the documented ordering
+ # Deterministic order: ranks are strictly increasing per state. + by_state: dict[tuple[int, int], list[int]] = {} + for p in proposals: + by_state.setdefault((p["step"], p["beam_row"]), []).append(p["rank"]) + for ranks in by_state.values(): + assert ranks == sorted(ranks) and len(ranks) == len(set(ranks)) for p in proposals: assert set(p) >= { "step", "beam_row", "rank", "action", "action_name", "score", "applicable", "rejection_reason", "selected", "consumed_budget", } - # Deterministic order: ranks are strictly increasing per state. if p["applicable"]:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_harnesses/experiments/test_slm310_action_alignment.py` around lines 151 - 157, In the test block iterating over each state’s predictions, enforce the documented deterministic ordering by asserting that successive applicable prediction ranks are strictly increasing. Track the previous rank within each state and update it as predictions are processed; keep the existing rejection-reason assertions unchanged.scripts/run_slm308_distance_value.py-614-616 (1)
614-616: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
--md-outcrashes when its parent directory doesn't exist.Only
json_out.parentis created. A--md-outpointing outsidedocs/designraisesFileNotFoundErrorafter the (expensive) two-arm run has already completed.🛡️ Proposed fix
args.json_out.parent.mkdir(parents=True, exist_ok=True) args.json_out.write_text(json.dumps(payload, indent=2) + "\n", encoding="utf-8") + args.md_out.parent.mkdir(parents=True, exist_ok=True) args.md_out.write_text(render_markdown(payload), encoding="utf-8")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/run_slm308_distance_value.py` around lines 614 - 616, Update the output-writing flow in the script’s main execution path to create args.md_out.parent with parents=True and exist_ok=True before calling write_text, matching the existing json_out.parent setup. Preserve the current markdown payload and encoding behavior.docs/design/iter-slm310-action-alignment-20260724.json-3022-3036 (1)
3022-3036: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winQualify
stop_slot_correctedas a non-inferiority result.
cell_cusesstop: "corrected"and the corrected path reaches budget accounting, butcell_cis identical tocell_aexcept for the 73/74 STOP budget delta. Because the artifact’s “adopted” verdict is based on thresholds that allow equality, reflect that this is a non-inferiority result, not a demonstrated improvement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/iter-slm310-action-alignment-20260724.json` around lines 3022 - 3036, Update the t3_stop_accounting result and related adopted verdict metadata to explicitly classify stop_slot_corrected as non-inferior rather than an improvement, since cell_c and cell_a are otherwise identical and the 73/74 budget delta only satisfies the equality-permitting threshold. Preserve the existing budget and valid_final checks.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1c18f6bf-8208-45c7-8f8c-4d46ac199b27
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (42)
README.mddocs/design/agentv-evaluation.mddocs/design/iter-slm303-decode-budget-audit-20260724.jsondocs/design/iter-slm303-decode-budget-audit-20260724.mddocs/design/iter-slm305-edit-language-20260724.jsondocs/design/iter-slm305-edit-language-20260724.mddocs/design/iter-slm308-distance-value-20260724.jsondocs/design/iter-slm308-distance-value-20260724.mddocs/design/iter-slm310-action-alignment-20260724.jsondocs/design/iter-slm310-action-alignment-20260724.mddocs/design/iter-slm312-state-sources-20260724.jsondocs/design/iter-slm312-state-sources-20260724.mddocs/design/langsmith-telemetry-smoke-20260724.jsonpyproject.tomlscripts/run_agentv_eval.mjsscripts/run_slm299_reachability_audit.pyscripts/run_slm303_decode_budget_audit.pyscripts/run_slm308_distance_value.pyscripts/run_slm310_action_alignment.pyscripts/run_slm312_state_sources.pysrc/slm_training/evals/agentv.pysrc/slm_training/harnesses/experiments/slm299_edit_reachability.pysrc/slm_training/harnesses/experiments/slm308_distance_oracle.pysrc/slm_training/harnesses/experiments/slm310_action_alignment.pysrc/slm_training/harnesses/experiments/slm312_state_sources.pysrc/slm_training/harnesses/model_build/decode_outcome.pysrc/slm_training/harnesses/model_build/eval_runner.pysrc/slm_training/harnesses/model_build/ship_gates.pysrc/slm_training/models/checkpoint_migrate.pysrc/slm_training/models/tree_edit_diffusion.pysrc/slm_training/resources/versions.jsonsrc/slm_training/runtime/telemetry/trace.pytests/test_evals/test_agentv.pytests/test_harnesses/experiments/test_slm299_edit_reachability.pytests/test_harnesses/experiments/test_slm308_distance_value.pytests/test_harnesses/experiments/test_slm310_action_alignment.pytests/test_harnesses/experiments/test_slm312_state_sources.pytests/test_harnesses/model_build/test_decode_outcome.pytests/test_harnesses/model_build/test_eval_gates.pytests/test_models/test_tree_edit_diffusion.pytests/test_runtime_trace.pytests/test_scripts/test_run_slm303_decode_budget_audit.py
| "version_stamp": { | ||
| "stamp_schema": "version_stamp/v1", | ||
| "code_commit": "5d35b90d7fc58c094604c23d702ab65593f98004", | ||
| "code_dirty": true, | ||
| "components": { | ||
| "harness.experiments.slm299_edit_reachability": "v2" | ||
| }, | ||
| "stamped_at": "2026-07-24T20:22:57.129778+00:00" | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Current registry version + history for the reachability component.
fd -t f 'versions.json' -p src | xargs -I{} jq '.components["harness.experiments.slm299_edit_reachability"]' {}
# Every artifact that stamps this component, with its stamped version.
fd -e json . docs/design --exec sh -c '
v=$(jq -r ".version_stamp.components[\"harness.experiments.slm299_edit_reachability\"] // empty" "$1" 2>/dev/null)
[ -n "$v" ] && printf "%s\t%s\t%s\n" "$v" "$(jq -r ".version_stamp.stamped_at // \"?\"" "$1")" "$1"
' sh {} | sortRepository: Tyler-R-Kendrick/slm-training
Length of output: 167
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Tracked versions.json files:\n'
git ls-files | grep -E '(^|/)versions\.json$' || true
printf '\nComponent value in tracked versions.json files:\n'
git ls-files | grep -E '(^|/)versions\.json$' | while IFS= read -r f; do
echo "--- $f"
jq -c '.components["harness.experiments.slm299_edit_reachability"]' "$f" 2>/dev/null || true
done
printf '\njson artifacts under docs/design stamping the component:\n'
git ls-files 'docs/design/*.json' | python3 - <<'PY'
import json, subprocess, sys, pathlib
for path in sys.stdin:
path=path.strip()
data=json.load(open(path, encoding='utf-8'))
comps=data.get("version_stamp",{}).get("components",{})
val=comps.get("harness.experiments.slm299_edit_reachability")
if val:
stamp=(data.get("version_stamp",{}).get("stamped_at") or "?")
print(f"{val}\t{stamp}\t{path}")
PY | sort
printf '\nJSON artifact sizes and relevant stamped_at/version lines:\n'
for f in $(git ls-files 'docs/design/*.json'); do
wc -l "$f"
doneRepository: Tyler-R-Kendrick/slm-training
Length of output: 167
🏁 Script executed:
#!/bin/bash
set +e
printf 'pwd: '; pwd
printf 'repo root files: '; ls -la | head -40
printf '\nTracked files matching versions/version/stamp/design:\n'
git ls-files | grep -Ei '(version|stamp|design|slm299|slm305|slm308|slm310)' | head -200 || true
printf '\nSearch version_stamp.components around slm299_edit_reachability:\n'
grep -R --include='*.json' --include='*.md' --include='*.py' --include='*.txt' 'slm299_edit_reachability' . | head -100 || trueRepository: Tyler-R-Kendrick/slm-training
Length of output: 167
🏁 Script executed:
#!/bin/bash
set -e
printf 'Directory listing with no filters and short entries:\n'
ls -A | sed -n '1,120p'
printf '\nFind matching tracked paths (raw):\n'
git ls-files 2>/dev/null | grep -Ei 'versions|version|slm299|slm305|slm308|slm310|design' | sed -n '1,200p' || true
printf '\nFind matching paths that look like json files (grep not git ls-files):\n'
find . -maxdepth 4 -type f -print 2>/dev/null | grep -Ei 'versions|version|slm299|slm305|slm308|slm310|design.*json' | sed -n '1,200p' || trueRepository: Tyler-R-Kendrick/slm-training
Length of output: 167
🏁 Script executed:
#!/bin/bash
set +e
printf 'Working copy root listing (first 80 entries):\n'
find . -mindepth 1 -maxdepth 3 -type f -print 2>/dev/null | sort | sed -n '1,80p'
printf '\nTracked top-level files:\n'
git ls-files 2>/dev/null | sort | sed -n '1,120p'Repository: Tyler-R-Kendrick/slm-training
Length of output: 167
Re-run this reachability audit at the final component version before merge.
The component registry shows the reachability component has moved past v2. Regenerate docs/design/iter-slm305-edit-language-20260724.json and its matching Markdown from scripts/run_slm299_reachability_audit.py so the reason codes, invariants, and old_vs_extended table match the latest code path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/iter-slm305-edit-language-20260724.json` around lines 306 - 314,
Regenerate the reachability audit artifacts using
scripts/run_slm299_reachability_audit.py before merging. Update the component
version in the version_stamp entry and ensure
docs/design/iter-slm305-edit-language-20260724.json plus its matching Markdown
reflect the latest reason codes, invariants, and old_vs_extended table.
Source: Coding guidelines
| function langsmithEnabled() { | ||
| return /^(1|true|yes|on)$/i.test(process.env.LANGSMITH_TRACING ?? "") | ||
| && Boolean(process.env.LANGSMITH_API_KEY); | ||
| } | ||
|
|
||
| function traceUuid(traceId) { | ||
| return traceId && /^[0-9a-f]{32}$/i.test(traceId) | ||
| ? `${traceId.slice(0, 8)}-${traceId.slice(8, 12)}-${traceId.slice(12, 16)}-${traceId.slice(16, 20)}-${traceId.slice(20)}` | ||
| : undefined; | ||
| } | ||
|
|
||
| async function publishLangSmithSummary({ traceId, runId, experiment, result }) { | ||
| const parentRunId = traceUuid(traceId); | ||
| if (!langsmithEnabled() || !parentRunId) return; | ||
| try { | ||
| const now = new Date().toISOString(); | ||
| const endpoint = `${(process.env.LANGSMITH_ENDPOINT || "https://api.smith.langchain.com").replace(/\/$/, "")}/runs`; | ||
| const response = await fetch(endpoint, { | ||
| method: "POST", | ||
| headers: { | ||
| "content-type": "application/json", | ||
| "x-api-key": process.env.LANGSMITH_API_KEY, | ||
| ...(process.env.LANGSMITH_WORKSPACE_ID | ||
| ? { "x-tenant-id": process.env.LANGSMITH_WORKSPACE_ID } | ||
| : {}), | ||
| }, | ||
| body: JSON.stringify({ | ||
| id: randomUUID(), | ||
| trace_id: parentRunId, | ||
| parent_run_id: parentRunId, | ||
| project_name: process.env.LANGSMITH_PROJECT || "slm-training", | ||
| name: "agentv.publication", | ||
| run_type: "tool", | ||
| inputs: { run_id: runId, experiment }, | ||
| outputs: { summary: result.summary }, | ||
| start_time: now, | ||
| end_time: now, | ||
| extra: { metadata: { w3c_trace_id: traceId, sdk: "@agentv/core" } }, | ||
| }), | ||
| signal: AbortSignal.timeout(500), | ||
| }); | ||
| if (!response.ok) throw new Error(`HTTP ${response.status}`); | ||
| } catch (error) { | ||
| console.warn(`LangSmith AgentV export failed: ${String(error)}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
New non-trivial logic lacks a runnable check.
traceUuid, langsmithEnabled, and publishLangSmithSummary introduce validation and network-request-construction logic with no accompanying test in this cohort. As per coding guidelines, "Non-trivial logic must leave one runnable check behind, such as an assert-based self-check or one small test file; trivial one-liners do not require tests."
#!/bin/bash
# Check for existing coverage of the new LangSmith export logic
fd -e mjs -e js -e ts . tests 2>/dev/null | xargs rg -l 'run_agentv_eval|publishLangSmithSummary|traceUuid' 2>/dev/null
rg -n 'run_agentv_eval' -g '*.{md,yml,yaml,json}'If no test exists, consider a small self-check (e.g., asserting traceUuid output shape and that publishLangSmithSummary no-ops when disabled/invalid) alongside this file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/run_agentv_eval.mjs` around lines 19 - 64, Add a runnable
assert-based self-check or focused test covering the new LangSmith logic in
traceUuid, langsmithEnabled, and publishLangSmithSummary. Validate UUID
formatting and that publishLangSmithSummary exits without a network request when
tracing is disabled or the trace ID is invalid, using the file’s existing
execution/test conventions.
Source: Coding guidelines
| body: JSON.stringify({ | ||
| id: randomUUID(), | ||
| trace_id: parentRunId, | ||
| parent_run_id: parentRunId, | ||
| project_name: process.env.LANGSMITH_PROJECT || "slm-training", | ||
| name: "agentv.publication", | ||
| run_type: "tool", | ||
| inputs: { run_id: runId, experiment }, | ||
| outputs: { summary: result.summary }, | ||
| start_time: now, | ||
| end_time: now, | ||
| extra: { metadata: { w3c_trace_id: traceId, sdk: "@agentv/core" } }, | ||
| }), |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== RTK availability =="
if command -v rtk >/dev/null 2>&1; then
rtk --version || true
else
echo "rtk not on PATH"
fi
echo "== Locate relevant files =="
git ls-files | rg '(^|/)scripts/run_agentv_eval\.mjs$|eval_runner\.py$|package(-lock)?\.json$|pnpm-lock\.yaml$|yarn\.lock$|package.json$' || true
echo "== Inspect JS wrapper relevant section =="
if [ -f scripts/run_agentv_eval.mjs ]; then
nl -ba scripts/run_agentv_eval.mjs | sed -n '1,140p'
fi
echo "== Inspect Python langsmith helper/tests =="
if [ -f scripts/eval_runner.py ]; then
rg -n "_record_langsmith_evaluation|LANGSMITH|safe|prompt|metric|summary|record" scripts/eval_runner.py -C 4
nl -ba scripts/eval_runner.py | sed -n '1,260p'
fi
echo "== Search for eval result handling =="
rg -n "evaluate\\(|result\\.summary|summary:" . --glob '!node_modules' --glob '!dist' --glob '!build' -C 2 || true
echo "== Package dependency versions =="
for f in package.json scripts/package.json package-lock.json pnpm-lock.yaml yarn.lock; do
[ -f "$f" ] && { echo "-- $f"; rg -n '`@agentv/core`|langsmith|langchain|node_modules' "$f" -C 2 || true; }
doneRepository: Tyler-R-Kendrick/slm-training
Length of output: 2052
🌐 Web query:
@agentv/core 4.42.4 evaluate result.summary fields
💡 Result:
In the @agentv/core package (and the associated @agentv/sdk), the evaluate function returns an object containing a results array and a summary object [1][2]. The summary object provides an aggregate overview of the evaluation run [1][3]. Based on the AgentV documentation and examples, the summary object typically includes at least the following fields [1]: - passed: The number of test cases that passed [1]. - total: The total number of test cases executed [1]. These fields are used to track and report the success rate of the evaluation suite [1]. When using the programmatic SDK, developers often access these fields to log the results to the console (e.g., console.log(${summary.passed}/${summary.total} passed)) [1][2]. Additionally, when AgentV runs evaluations, it generates a summary.json file in the output directory, which acts as the canonical aggregate summary for that run [3]. While the programmatic summary object focuses on pass/total counts, the on-disk artifacts and broader evaluation engine provide deeper metrics and metadata for audit and reporting [3].
Citations:
- 1: https://agentv.dev/docs/evaluation/sdk/
- 2: https://github.com/EntityProcess/agentv
- 3: https://agentv.dev/docs/evaluation/running-evals/
🌐 Web query:
EntityProcess agentv sdk evaluate summary metadata prompts inputs outputs
💡 Result:
AgentV is a light-weight TypeScript-based framework for evaluating and optimizing AI agents, developed by EntityProcess [1][2]. While @agentv/sdk and @agentv/core were previously used as primary surfaces, the project has consolidated its public authoring interface under the agentv package to provide a streamlined, facade-based API [3]. Evaluation in AgentV can be performed programmatically via the evaluate() function or through YAML-based configuration files [1][4]. Key concepts include: - Evaluate API: The evaluate() function allows for programmatic execution of test cases [1][5]. It accepts inputs (such as test suites or prompt matrices), executes them against defined agent targets, and returns a result object containing a summary of performance (e.g., passed/total) [1][6]. It also supports writing canonical run artifacts to disk [6]. - Metadata: AgentV supports a structured metadata block within evaluation specifications (often YAML), which can include fields like name and description [7]. Metadata is used for organization and reporting in evaluation suites [7]. - Prompts & Inputs: The framework supports complex input management, including top-level prompts for defining input matrices [8]. This allows for the systematic expansion of prompt instances during evaluation, where each instance is tracked with a unique identity for precise comparison and regression detection [8]. - Outputs & Assertions: The framework validates agent behavior through assertions (e.g., contains, equals, regex) [1][7]. These can be defined deterministically in code (using defineCodeGrader()) or as structured YAML assertions [1][4][7]. The system evaluates these against the agent's output to determine if the result meets the specified criteria [1][4]. For most users, the recommended way to interact with the framework is by installing and importing the agentv package, which acts as the canonical facade for CLI operations and programmatic evaluation tasks [3]. Documentation and examples for authoring custom graders, assertions, and evaluations are available in the project repository [4].
Citations:
- 1: https://github.com/EntityProcess/agentv
- 2: https://www.npmjs.com/package/@agentv/core
- 3: feat(package): expose agentv authoring facade EntityProcess/agentv#1727
- 4: https://github.com/EntityProcess/agentv/blob/main/examples/README.md
- 5: EntityProcess/agentv@a64d2ee
- 6: feat(sdk): let evaluate write canonical run artifacts EntityProcess/agentv#1410
- 7: EntityProcess/agentv@7ee118c
- 8: feat(evals): add prompt instance expansion EntityProcess/agentv#1602
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Repository files available =="
(python3 - <<'PY'
import os
for root, dirs, files in os.walk('.'):
dirs[:] = [d for d in dirs if d not in {'.git','node_modules','dist','build','.agents'}]
for f in files:
if f in {'scripts/run_agentv_eval.mjs','scripts/eval_runner.py','package.json','package-lock.json','pnpm-lock.yaml','yarn.lock'}:
path=os.path.join(root,f)
print(path)
py
) 2>/dev/null || true
echo "== Target files =="
for f in scripts/run_agentv_eval.mjs scripts/eval_runner.py package.json package-lock.json pnpm-lock.yaml yarn.lock; do
if [ -f "$f" ]; then
echo "-- $f --"
rg -n "`@agentv/core`|evaluate\\(|result\\.summary|_record_langsmith_evaluation|summary:|prompt|outputs|inputs" "$f" -C 3 || true
else
echo "-- missing $f --"
fi
done
echo "== Candidate package manifests =="
python3 - <<'PY'
import os,json
for root, dirs, files in os.walk('.'):
dirs[:] = [d for d in dirs if d not in {'.git','node_modules','dist','build','.agents'}]
for f in files:
if f.endswith('package.json'):
p=os.path.join(root,f)
try:
data=json.load(open(p))
except Exception as e:
print(f"{p}: json error {e}")
continue
if '`@agentv/core`' in data.get('dependencies',{}) or '`@agentv/core`' in data.get('devDependencies',{}):
print(p)
print(dict(data.get('dependencies',{}), **data.get('devDependencies',{})))
PYRepository: Tyler-R-Kendrick/slm-training
Length of output: 2052
🌐 Web query:
github EntityProcess agentv evaluate result.summary source
💡 Result:
In the context of the GitHub project EntityProcess/agentv, the evaluate function returns an object containing results and summary [1]. The summary object provides aggregated evaluation metrics, allowing developers to track performance, such as the number of passed tests versus total tests [1]. For example, when using the AgentV TypeScript SDK, the usage is as follows: const { results, summary } = await evaluate({ tests: [ { id: 'greeting', input: 'Say hello', assertions: [{ type: 'contains', value: 'Hello' }], }, ], }); console.log(${summary.passed}/${summary.total} passed); AgentV is a lightweight AI agent evaluation framework that enables users to run evaluation cases against AI agents, scoring them using a combination of deterministic code graders and customizable LLM graders [1]. It is designed to be CI/CD native, producing JSONL outputs and supporting version-controlled evaluation workflows [1]. Note that while other systems (such as NVIDIA NeMo Platform and Neo4j) also utilize objects named result.summary for reporting, they are distinct from the AgentV framework [2][3][4].
Citations:
- 1: https://github.com/EntityProcess/agentv
- 2: https://docs.nvidia.com/nemo-platform/documentation/evaluate-models/agent-eval/reading-results
- 3: https://neo4j.com/docs/java-manual/current/result-summary/
- 4: https://neo4j.com/docs/javascript-manual/5/result-summary/
🌐 Web query:
site:github.com/EntityProcess/agentv "result.summary" "@agentv/core"
💡 Result:
@agentv/core is a private, low-level internal package within the AgentV project, which is a framework for lightweight AI agent evaluation and optimization [1][2]. It serves as the underlying engine, providing core contracts, runners, and artifact-handling logic used by the AgentV CLI, Dashboard, tests, and other internal systems [1][3]. While older documentation or examples may show direct imports from @agentv/core for programmatic usage [2], recent project developments have established it as an internal-only workspace layer [1]. For public-facing programmatic usage and authoring, users are now directed to use the canonical facade package, agentv, or the @agentv/sdk package [1]. The SDK provides the intended TypeScript APIs, such as defineCodeGrader and evaluate, acting as the public interface while routing requests to the underlying core engine [1][4][5].
Citations:
- 1: feat(package): expose agentv authoring facade EntityProcess/agentv#1727
- 2: https://github.com/EntityProcess/agentv
- 3: feat(sdk): let evaluate write canonical run artifacts EntityProcess/agentv#1410
- 4: feat(sdk): expose evaluate api EntityProcess/agentv#1485
- 5: https://github.com/EntityProcess/agentv/tree/main/examples
🌐 Web query:
Node @agentv/sdk evaluate returns results summary pass total types
💡 Result:
In the @agentv/sdk, the evaluate function is used to programmatically run evaluations and returns an object containing results and a summary of the test execution [1]. The summary object provides metrics to track the evaluation progress, most notably the passed and total fields [1][2][3]. Based on the SDK's programmatic API pattern, these types are typically: - passed: number (the count of tests that passed) [1][2][3] - total: number (the total count of tests executed) [1][2][3] You can access these values directly from the object returned by the evaluate function [1][3]: import { evaluate } from '@agentv/sdk'; const { results, summary } = await evaluate({ tests: [ { id: 'example-test', input: 'Say hello', expectedOutput: 'Hello there!', assertions: [{ type: 'contains', value: 'Hello' }], }, ], }); console.log(${summary.passed}/${summary.total} passed); The @agentv/sdk package serves as the primary user-facing entrypoint for this functionality, re-exporting the evaluate API from @agentv/core [1][4]. The API supports both running existing YAML/JSONL specification files via the specFile option or defining tests inline using the tests array [1].
Citations:
- 1: https://agentv.dev/docs/evaluation/sdk/
- 2: https://github.com/EntityProcess/agentv
- 3: https://registry.npmjs.org/agentv
- 4: feat(sdk): expose evaluate api EntityProcess/agentv#1485
🌐 Web query:
npm @agentv/core 4.42.4 types dist
💡 Result:
The @agentv/core package is a legitimate TypeScript-based AI agent evaluation and optimization framework available on npm [1][2]. Version 4.42.4 was published on June 19, 2026 [3][1]. As a low-level package, @agentv/core provides the core evaluation engine and runtime primitives for the AgentV ecosystem [1][4]. It is designed primarily for programmatic usage or custom integrations [1][4], while most users are directed to use the agentv CLI package for general tasks [1][4]. Regarding the types and distribution (dist) structure: 1. Types: As a TypeScript-authored package, @agentv/core includes its own type definitions. These are typically bundled within the package, allowing for seamless integration in TypeScript projects where you can import components like createProvider or runEvaluation directly [1][4]. 2. Dist: The package follows standard npm distribution practices. Files are bundled in the package's dist directory (or equivalent output directory defined in its build configuration) when installed via npm. This typically includes the transpiled JavaScript files and corresponding declaration files (.d.ts) to support TypeScript intellisense and type checking. For technical details or specific file structures for version 4.42.4, you can view the source code and build artifacts directly in the official GitHub repository at EntityProcess/agentv [3][1][5].
Citations:
- 1: https://www.npmjs.com/package/@agentv/core
- 2: https://registry.npmjs.org/@agentv/core
- 3: https://github.com/EntityProcess/agentv/releases/tag/v4.42.4
- 4: https://npmx.dev/package/@agentv/core
- 5: https://github.com/EntityProcess/agentv
Whitelist summary fields before exporting to LangSmith.
result.summary should not be treated as a safe third-party payload by default. Use the same curated, metric-only shape as _record_langsmith_evaluation in eval_runner.py; send only the expected fields such as pass_count/total_count, not raw result.summary contents.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/run_agentv_eval.mjs` around lines 45 - 57, Update the LangSmith
payload in the run-agent evaluation flow to replace raw result.summary export
with a curated metric-only summary matching _record_langsmith_evaluation in
eval_runner.py. Explicitly whitelist the expected fields, such as pass_count and
total_count, and preserve the existing outputs structure without forwarding
additional result.summary contents.
| if fallback_count is None: | ||
| classification = "unmeasured" | ||
| elif timeout_count: | ||
| classification = "runtime_timeout_interference" | ||
| elif fallback_count: | ||
| classification = "fallback_interference" | ||
| else: | ||
| classification = "model_behavior" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
unmeasured outranks a recorded timeout, discarding a real budget fact.
fallback_count is None is checked first, so a historical scoreboard with decode_timeout_count=3 but fallback_count=null classifies as unmeasured rather than runtime_timeout_interference. eval_runner writes fallback_count = None whenever the plugin exposes no decode stats, so that combination is expected for precisely the older boards this audit targets — and _disposition (Lines 686-690, 708-719) then emits no timeout annotation and no interference count for them, which is the audit's central output.
A recorded nonzero timeout is affirmative evidence; missing fallback telemetry only makes the fallback dimension unmeasured.
🐛 Suggested precedence fix
- if fallback_count is None:
- classification = "unmeasured"
- elif timeout_count:
+ if timeout_count:
classification = "runtime_timeout_interference"
+ elif fallback_count is None:
+ classification = "unmeasured"
elif fallback_count:
classification = "fallback_interference"
else:
classification = "model_behavior"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if fallback_count is None: | |
| classification = "unmeasured" | |
| elif timeout_count: | |
| classification = "runtime_timeout_interference" | |
| elif fallback_count: | |
| classification = "fallback_interference" | |
| else: | |
| classification = "model_behavior" | |
| if timeout_count: | |
| classification = "runtime_timeout_interference" | |
| elif fallback_count is None: | |
| classification = "unmeasured" | |
| elif fallback_count: | |
| classification = "fallback_interference" | |
| else: | |
| classification = "model_behavior" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/run_slm303_decode_budget_audit.py` around lines 169 - 176, Update the
classification precedence in the visible classification logic so a nonzero
timeout_count is checked before fallback_count is None, yielding
"runtime_timeout_interference" whenever recorded timeouts exist. Preserve
"unmeasured" only for cases without timeouts and missing fallback telemetry, and
keep the existing fallback_interference and model_behavior outcomes for their
respective conditions.
| ctx = mp.get_context("fork") | ||
| queue: Any = ctx.Queue() | ||
| proc = ctx.Process( | ||
| target=_cell_worker, args=(checkpoint_path, budget, record_id, queue) | ||
| ) | ||
| t0 = time.perf_counter() | ||
| proc.start() | ||
| proc.join(float(budget["timeout_s"])) | ||
| elapsed_ms = round((time.perf_counter() - t0) * 1000.0, 2) | ||
| if proc.is_alive(): | ||
| proc.terminate() | ||
| proc.join(30) | ||
| outcome = classify_decode_outcome( | ||
| parse_ok=None, timed_out=True, fallback_counters=0 | ||
| ) | ||
| cell = { | ||
| "status": "decode_timeout", | ||
| "outcome": outcome, | ||
| "stop_reason": "decode_timeout", | ||
| "elapsed_ms": elapsed_ms, | ||
| "prediction": None, | ||
| "parse_ok": None, | ||
| } | ||
| else: | ||
| result = ( | ||
| queue.get() | ||
| if not queue.empty() | ||
| else { | ||
| "status": "harness_error", | ||
| "error": f"worker exited without result (code {proc.exitcode})", | ||
| } | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
join() before draining the queue can fabricate a timeout, and queue.empty() can lose a real result.
Two ordering hazards in the standard multiprocessing.Queue contract:
- A child that has
put()a payload larger than the pipe buffer stays alive until the parent drains it. Because the parent callsproc.join(timeout_s)before touching the queue, that child is still alive at Line 512, so a successful decode is recorded asstatus="decode_timeout"withoutcome="runtime_timeout"— fabricated budget evidence, which is exactly what this audit is supposed to distinguish. Predictions at the 10x cap (2560 tokens) are the arm most likely to trip it. queue.empty()(Line 529) is advisory: it can report empty while the item is still in the child's feeder thread even after a clean exit, converting a real result intoharness_error/ "worker exited without result".
Also, terminate() sends SIGTERM only; a wedged native decode can ignore it, leaving the parent blocked for 30s and an orphan behind.
🐛 Suggested fix: drain first, then join, then escalate
ctx = mp.get_context("fork")
queue: Any = ctx.Queue()
proc = ctx.Process(
target=_cell_worker, args=(checkpoint_path, budget, record_id, queue)
)
t0 = time.perf_counter()
proc.start()
- proc.join(float(budget["timeout_s"]))
- elapsed_ms = round((time.perf_counter() - t0) * 1000.0, 2)
- if proc.is_alive():
- proc.terminate()
- proc.join(30)
+ # Read the result FIRST: a child blocked flushing a large payload never
+ # exits, so joining before draining would misreport it as a timeout.
+ import queue as _queue
+
+ result: dict[str, Any] | None
+ try:
+ result = queue.get(timeout=float(budget["timeout_s"]))
+ except _queue.Empty:
+ result = None
+ elapsed_ms = round((time.perf_counter() - t0) * 1000.0, 2)
+ proc.join(30 if result is not None else 0)
+ if proc.is_alive():
+ proc.terminate()
+ proc.join(10)
+ if proc.is_alive():
+ proc.kill()
+ proc.join(10)
+ if result is None:
outcome = classify_decode_outcome(
parse_ok=None, timed_out=True, fallback_counters=0
)
@@
else:
- result = (
- queue.get()
- if not queue.empty()
- else {
- "status": "harness_error",
- "error": f"worker exited without result (code {proc.exitcode})",
- }
- )
if result["status"] == "ok":📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ctx = mp.get_context("fork") | |
| queue: Any = ctx.Queue() | |
| proc = ctx.Process( | |
| target=_cell_worker, args=(checkpoint_path, budget, record_id, queue) | |
| ) | |
| t0 = time.perf_counter() | |
| proc.start() | |
| proc.join(float(budget["timeout_s"])) | |
| elapsed_ms = round((time.perf_counter() - t0) * 1000.0, 2) | |
| if proc.is_alive(): | |
| proc.terminate() | |
| proc.join(30) | |
| outcome = classify_decode_outcome( | |
| parse_ok=None, timed_out=True, fallback_counters=0 | |
| ) | |
| cell = { | |
| "status": "decode_timeout", | |
| "outcome": outcome, | |
| "stop_reason": "decode_timeout", | |
| "elapsed_ms": elapsed_ms, | |
| "prediction": None, | |
| "parse_ok": None, | |
| } | |
| else: | |
| result = ( | |
| queue.get() | |
| if not queue.empty() | |
| else { | |
| "status": "harness_error", | |
| "error": f"worker exited without result (code {proc.exitcode})", | |
| } | |
| ) | |
| ctx = mp.get_context("fork") | |
| queue: Any = ctx.Queue() | |
| proc = ctx.Process( | |
| target=_cell_worker, args=(checkpoint_path, budget, record_id, queue) | |
| ) | |
| t0 = time.perf_counter() | |
| proc.start() | |
| import queue as _queue | |
| result: dict[str, Any] | None | |
| try: | |
| result = queue.get(timeout=float(budget["timeout_s"])) | |
| except _queue.Empty: | |
| result = None | |
| elapsed_ms = round((time.perf_counter() - t0) * 1000.0, 2) | |
| proc.join(30 if result is not None else 0) | |
| if proc.is_alive(): | |
| proc.terminate() | |
| proc.join(10) | |
| if proc.is_alive(): | |
| proc.kill() | |
| proc.join(10) | |
| if result is None: | |
| outcome = classify_decode_outcome( | |
| parse_ok=None, timed_out=True, fallback_counters=0 | |
| ) | |
| cell = { | |
| "status": "decode_timeout", | |
| "outcome": outcome, | |
| "stop_reason": "decode_timeout", | |
| "elapsed_ms": elapsed_ms, | |
| "prediction": None, | |
| "parse_ok": None, | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/run_slm303_decode_budget_audit.py` around lines 503 - 534, Update the
worker lifecycle around _cell_worker to drain the multiprocessing queue before
joining the child, using a bounded retrieval strategy that does not rely on
queue.empty(). After draining, join within the decode timeout and classify only
genuinely unfinished workers as decode_timeout; if termination is required,
escalate after terminate when the process remains alive so no orphan or extra
30-second block remains. Preserve the existing result payload and harness_error
handling for workers that exit without producing a result.
| MEANINGFUL_METRIC_POLICY = { | ||
| "active_primary": "meaningful_program_v1", | ||
| "threshold_version": "openui_ship_gates_v2", | ||
| "threshold_version": "openui_ship_gates_v3", |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Enumerate every ship-gate policy identity spelling and its source.
set -u
rg -n 'openui_ship_gates_v[0-9]+' --glob '!**/node_modules/**' -g '!**/*.lock'
rg -n -C3 'threshold_version' --type=pyRepository: Tyler-R-Kendrick/slm-training
Length of output: 167
🏁 Script executed:
#!/bin/bash
set -u
printf 'Tracked files matching ship_gates.py/versions.json:\n'
fd -a 'ship_gates.py|versions.json' . | sed 's#^\./##'
printf '\nSearch policy identities (no git return-code sensitivity):\n'
rg -n 'openui_ship_gates_v[0-9]+' . || true
printf '\nSearch threshold_version/Meaningful policy:\n'
rg -n -C3 'threshold_version|Meaningful|meaningful_metric_policy|policy_id' . || true
printf '\nGit status stat:\n'
git status --short || trueRepository: Tyler-R-Kendrick/slm-training
Length of output: 50386
🏁 Script executed:
#!/bin/bash
set -u
echo 'Find Python/JS tracked file candidates excluding likely huge generated bundles:'
fd -a 'ship_gates.py|versions.json|package-lock.json|yarn.lock|pnpm-lock.yaml' . 2>/dev/null | sed 's#^\./##' | while read -r f; do
case "$f" in
*node_modules*|*static/app/assets*|*dist*|*build*) continue ;;
*.lock|*package-lock.json|*yarn.lock|*pnpm-lock.yaml*) continue ;;
*) echo "$f" ;;
esac
done
echo
echo 'Inspect src/slm_training/harnesses/model_build/ship_gites.py outline and relevant section:'
if [ -f src/slm_training/harnesses/model_build/ship_gates.py ]; then
ast-grep outline src/slm_training/harnesses/model_build/ship_gates.py --view compact || true
wc -l src/slm_training/harnesses/model_build/ship_gates.py
sed -n '1,180p' src/slm_training/harnesses/model_build/ship_gates.py | cat -n
else
echo 'src/slm_training/harnesses/model_build/ship_gates.py not found'
fi
echo
echo 'Focused policy identity search in text-based tracked source files only:'
python3 - <<'PY'
from pathlib import Path
import re
target = re.compile(r'openui_ship_gates_v\d+')
for path in Path('.').rglob('*'):
f = path.resolve()
s = repr(f)
if f.is_dir() or 'node_modules' in s or '.git' in s or any(part in s for part in ['.lock', 'node_modules']):
continue
if path.suffix in ('.py','.ts','.tsx','.js','.json','.yaml','.yml','.md','.mdx','.txt'):
try:
txt = path.read_text(errors='ignore').splitlines()
except Exception:
continue
for i,line in enumerate(txt,1):
if target.search(line):
print(f"{f}:{i}:{line.strip()}")
PY
echo
echo 'Focused threshold_version/policy_id search in tracked Python files:'
python3 - <<'PY'
from pathlib import Path
for path in Path('.').rglob('*.py'):
if 'node_modules' in str(path) or '.git' in str(path):
continue
try:
txt = path.read_text(errors='ignore')
except Exception:
continue
lines = txt.splitlines()
hits = [i for i,line in enumerate(lines,1) if 'threshold_version' in line or 'meaningful' in line.lower() or 'policy_id' in line]
if hits:
for i in hits[:50]:
lo=max(1,i-3); hi=min(len(lines),i+3)
print(f"\n--- {path}:{lo}-{hi} ---")
for n in range(lo,hi+1):
print(f"{n}:{lines[n-1]}")
PYRepository: Tyler-R-Kendrick/slm-training
Length of output: 50386
🏁 Script executed:
#!/bin/bash
set -u
printf 'Focused ship_gates.py line count and content:\n'
wc -l src/slm_training/harnesses/model_build/ship_gates.py 2>/dev/null || true
sed -n '1,180p' src/slm_training/harnesses/model_build/ship_gates.py 2>/dev/null | cat -n
printf '\nVersions gate entry:\n'
python3 - <<'PY'
from pathlib import Path
p=Path('src/slm_training/resources/versions.json')
if p.exists():
lines=p.read_text().splitlines()
for i,line in enumerate(lines,1):
if '"gates.ship"' in line or '"openui_ship_gates_v4"' in line or 'openui_ship_gates_v4' in line:
print(f"{i}:{line}")
lo=max(1,1198); hi=min(len(lines),1210)
print("\n--- versions.json 1198-1210 ---")
for n in range(lo,hi+1):
print(f"{n}:{lines[n-1]}")
PY
printf '\nRead-only parser of meaninful_metric_policy threshold_version assignment behavior:\n'
python3 - <<'PY'
from pathlib import Path
import ast, re
p=Path('src/slm_training/harnesses/model_build/ship_gates.py')
src=p.read_text()
tree=ast.parse(src)
for node in ast.walk(tree):
if isinstance(node, ast.FunctionDef) and node.name == '_meaningful_metric_policy':
print(f"function body lines {node.lineno}-{node.end_lineno}")
for i, n in enumerate(ast.walk(node),1):
if isinstance(n, ast.Assign):
for t in n.targets:
if isinstance(t, ast.Subscript) and isinstance(t.value, ast.Name) and t.value.id == 'threshold_version':
print(ast.dump(t.value, annotate_fields=False))
print('assigned value:')
print(ast.unparse(n.value))
print("\nfunction source:")
print(ast.get_source_segment(src, node))
PY
printf '\nRestricted source identity search excluding generated bundle directories using Path.walk:\n'
from pathlib import Path
import re
pat=re.compile(r'openui_ship_gates_v\d+')
for root, dirs, files in Path('.').walk(top_down=False):
dirs[:]=[d for d in dirs if d not in {'node_modules','.git'} and 'static' not in root.as_posix().replace('static/app/assets','') and 'dist' not in root.as_posix() and 'build' not in root.as_posix()]
for name in files:
if name.endswith(('.py','.ts','.tsx','.js','.json','.yaml','.yml','.md','.mdx','.txt','.lock')) or name in {'package.json','AGENTS.md','RTK.md'}:
f=root/name
if f.suffix in ('.py','.json','.js','.ts','.tsx','.yaml','.yml'):
txt=f.read_text(errors='ignore').splitlines()
else:
continue
for i,line in enumerate(txt,1):
if pat.search(line):
print(f"{f}:{i}:{line.strip()}")Repository: Tyler-R-Kendrick/slm-training
Length of output: 167
🏁 Script executed:
#!/bin/bash
set -u
printf 'Find ship_gates.py:\n'
find . -name 'ship_gates.py' -type f ! \( -path './node_modules/*' -o -path './.git/*' \) 2>/dev/null
printf '\nInspect candidate files including line count and excerpt:\n'
while IFS= read -r p; do
echo "===== $p ====="
wc -l "$p"
sed -n '1,180p' "$p" | cat -n
done < <(find . -name 'ship_gates.py' -type f ! \( -path './node_modules/*' -o -path './.git/*' \) 2>/dev/null)
printf '\nVersions gate entry snippets:\n'
while IFS= read -r p; do
echo "===== $p ====="
python3 - <<'PY' "$p"
import json, sys
from pathlib import Path
p=Path(sys.argv[1])
lines=p.read_text(errors='ignore').splitlines()
matches=[]
for i,line in enumerate(lines,1):
if '"gates.ship"' in line or '"openui_ship_gates_v4"' in line or 'openui_ship_gates_v4' in line or 'openui_ship_gates_v3' in line or 'openui_ship_gates_v2' in line:
matches.append((i,line))
for i,line in matches[:50]:
print(f"{i}:{line}")
# Best-effort JSON load of nearby object containing components only
try:
data=json.loads(p.read_text())
except Exception as exc:
print("json load failed:", exc)
raise SystemExit
print("top keys:", list(data.keys())[:20] if isinstance(data, dict) else type(data))
PY
done < <(find . -name 'versions.json' -type f ! \( -path './node_modules/*' -o -path './.git/*' \) 2>/dev/null)
printf '\nSource identity search excluding likely generated bundles:\n'
python3 - <<'PY'
from pathlib import Path
import re
pat=re.compile(r'openui_ship_gates_v\d+')
skip=set()
for root, dirs, files in Path('.').walk(top_down=False):
for d in list(dirs):
loc=root/d
if any(x in loc.as_posix() for x in {'node_modules','.git','static/app/assets','dist','build'}):
dirs.remove(d)
for name in files:
if name.endswith(('.py','.json','.js','.ts','.tsx','.yaml','.yml','.md','.mdx','.txt','.lock')):
f=root/name
try:
txt=f.read_text(errors='ignore').splitlines()
except Exception:
continue
for i,line in enumerate(txt,1):
if pat.search(line):
print(f"{f}:{i}:{line.strip()}")
PYRepository: Tyler-R-Kendrick/slm-training
Length of output: 171
🏁 Script executed:
#!/bin/bash
set -u
printf 'Top-level tracked files by name:\n'
git ls-files | awk -F/ '{print $1}' | sort -u | sed -n '1,200p'
printf '\nFiles containing ship_gates string:\n'
git ls-files | xargs rg -n 'ship_gates|meaningful_metric_policy' || true
printf '\nFiles containing versions entries:\n'
git ls-files | xargs rg -n 'gates\.ship|openui_ship_gates_v[0-9]+' || trueRepository: Tyler-R-Kendrick/slm-training
Length of output: 167
Reconcile the ship-gate policy version with the registry.
threshold_version is bumped to openui_ship_gates_v3, but the default threshold path still derives the emitted policy_id from the older ship-gates version while the registry’s gates.ship component is already openui_ship_gates_v4. Align the default policy_id with one source of truth (for example, the registry component version) so gate payloads are not backdated or split across policy ids.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/slm_training/harnesses/model_build/ship_gates.py` at line 69, The default
threshold path derives policy_id from an outdated ship-gates version,
conflicting with threshold_version and the registry’s gates.ship component.
Update the default policy_id logic to use the registry’s gates.ship version as
the single source of truth, ensuring emitted gate payloads consistently use the
current policy identifier.
| raw_config = dict(payload.get("config") or {}) | ||
| valid = set(TreeEditDiffusionConfig.__dataclass_fields__) | ||
| config = TreeEditDiffusionConfig( | ||
| **{key: value for key, value in raw_config.items() if key in valid} | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Migration silently flips legacy checkpoints to bounded_distance value labels.
from_checkpoint deliberately applies config_payload.setdefault("value_label_mode", "mutation_count") so pre-SLM-308 checkpoints keep their historical supervision semantics. This path filters raw_config but never applies that default, so a format-1 payload (which has no value_label_mode key) picks up the new dataclass default "bounded_distance", and model.save() then persists it — the migrated checkpoint claims a training regime it was never trained under. test_checkpoint_format2_fail_closed_and_migration cannot catch this because its simulated format-1 payload is derived from a format-2 save() and therefore already carries the key.
🐛 Preserve the historical label mode across migration
raw_config = dict(payload.get("config") or {})
+ # SLM-308 parity: checkpoints written before value_label_mode existed were
+ # trained with mutation-count labels (same rule as from_checkpoint).
+ raw_config.setdefault("value_label_mode", "mutation_count")
valid = set(TreeEditDiffusionConfig.__dataclass_fields__)
config = TreeEditDiffusionConfig(
**{key: value for key, value in raw_config.items() if key in valid}
)Consider extending the migration test to build the legacy payload with the new config keys removed, so the parity default is actually exercised.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| raw_config = dict(payload.get("config") or {}) | |
| valid = set(TreeEditDiffusionConfig.__dataclass_fields__) | |
| config = TreeEditDiffusionConfig( | |
| **{key: value for key, value in raw_config.items() if key in valid} | |
| ) | |
| raw_config = dict(payload.get("config") or {}) | |
| # SLM-308 parity: checkpoints written before value_label_mode existed were | |
| # trained with mutation-count labels (same rule as from_checkpoint). | |
| raw_config.setdefault("value_label_mode", "mutation_count") | |
| valid = set(TreeEditDiffusionConfig.__dataclass_fields__) | |
| config = TreeEditDiffusionConfig( | |
| **{key: value for key, value in raw_config.items() if key in valid} | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/slm_training/models/checkpoint_migrate.py` around lines 336 - 340, Update
the format-1 checkpoint migration path around raw_config and
TreeEditDiffusionConfig construction to apply the same "mutation_count"
value_label_mode default used by from_checkpoint when the legacy payload omits
that key. Preserve an explicitly stored value_label_mode, and extend
test_checkpoint_format2_fail_closed_and_migration to remove newer config keys
from the simulated legacy payload so the fallback is exercised.
| for rest_idx in range(len(CONTAINER_RESTS)): | ||
| scored.append( | ||
| ( | ||
| float(action_lp[ACTION_ADD_CONTAINER]) | ||
| + base | ||
| + float(comp_lp[comp]), | ||
| Edit(ACTION_ADD_CONTAINER, stmt, comp, target=rest_idx), | ||
| ) | ||
| ) | ||
| for slot in range(min(n_slots, MAX_SLOTS)): | ||
| slot_score = float(slot_lp[slot]) | ||
| scored.append( | ||
| ( | ||
| float(action_lp[ACTION_ADD]) | ||
| + base | ||
| + float(comp_lp[comp]) | ||
| + float(slot_lp[slot]), | ||
| + slot_score, | ||
| Edit(ACTION_ADD, stmt, comp, slot), | ||
| ) | ||
| ) | ||
| scored.append( | ||
| ( | ||
| float(action_lp[ACTION_BIND_PLACEHOLDER]) | ||
| + base | ||
| + slot_score, | ||
| Edit(ACTION_BIND_PLACEHOLDER, stmt, slot=slot), | ||
| ) | ||
| ) | ||
| for payload in leaf_comps: | ||
| for rest_idx in range(len(CONTAINER_RESTS)): | ||
| scored.append( | ||
| ( | ||
| float(action_lp[ACTION_INSERT_SUBTREE]) | ||
| + base | ||
| + float(comp_lp[comp]) | ||
| + slot_score, | ||
| Edit(ACTION_INSERT_SUBTREE, stmt, comp, slot, | ||
| target=rest_idx, payload=payload), | ||
| ) | ||
| ) | ||
| scored.append( | ||
| ( | ||
| float(action_lp[ACTION_REPLACE_SUBTREE]) | ||
| + base | ||
| + slot_score, | ||
| Edit(ACTION_REPLACE_SUBTREE, stmt, slot=slot, | ||
| payload=payload), | ||
| ) | ||
| ) | ||
| scored.append( | ||
| (float(action_lp[ACTION_REMOVE]) + base, Edit(ACTION_REMOVE, stmt)) | ||
| ) | ||
| scored.append( | ||
| ( | ||
| float(action_lp[ACTION_REMOVE_CONTAINER]) + base, | ||
| Edit(ACTION_REMOVE_CONTAINER, stmt), | ||
| ) | ||
| ) | ||
| for payload in range(len(V05_TEMPLATES)): | ||
| scored.append( | ||
| ( | ||
| float(action_lp[ACTION_REPLACE_STATEMENT]) + base, | ||
| Edit(ACTION_REPLACE_STATEMENT, stmt, payload=payload), | ||
| ) | ||
| ) | ||
| for payload in range(len(V05_TEMPLATES)): | ||
| scored.append( | ||
| ( | ||
| float(action_lp[ACTION_INSERT_STATEMENT]), | ||
| Edit(ACTION_INSERT_STATEMENT, payload=payload), | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
BIND_PLACEHOLDER and REPLACE_SUBTREE are enumerated inside the comp loop, emitting n_comp identical duplicates each.
Neither edit uses comp (both score off action_lp + base + slot_score only and construct Edit(...) without a component), yet they are appended once per component index. Consequences:
- The candidate list carries
n_comp × n_slots × (1 + len(leaf_comps))redundant entries per statement, all sorted on every decode step. - After the first copy is applied, each duplicate still costs a
space.applycall and is then recorded asduplicate_state— soverifier_calls,visited,dead_total, andvisited_sharesare all inflated. Those are exactly the SLM-310 metrics drivingdead_candidate_rate,action_calibration, and theloss_reweighting_prediction, so the recorded audit numbers are skewed toward the two duplicated actions.
🐛 Hoist the component-independent proposals out of the `comp` loop
for slot in range(min(n_slots, MAX_SLOTS)):
slot_score = float(slot_lp[slot])
scored.append(
(
float(action_lp[ACTION_ADD])
+ base
+ float(comp_lp[comp])
+ slot_score,
Edit(ACTION_ADD, stmt, comp, slot),
)
)
- scored.append(
- (
- float(action_lp[ACTION_BIND_PLACEHOLDER])
- + base
- + slot_score,
- Edit(ACTION_BIND_PLACEHOLDER, stmt, slot=slot),
- )
- )
for payload in leaf_comps:
for rest_idx in range(len(CONTAINER_RESTS)):
scored.append(
(
float(action_lp[ACTION_INSERT_SUBTREE])
+ base
+ float(comp_lp[comp])
+ slot_score,
Edit(ACTION_INSERT_SUBTREE, stmt, comp, slot,
target=rest_idx, payload=payload),
)
)
- scored.append(
- (
- float(action_lp[ACTION_REPLACE_SUBTREE])
- + base
- + slot_score,
- Edit(ACTION_REPLACE_SUBTREE, stmt, slot=slot,
- payload=payload),
- )
- )
+ # Component-independent proposals: enumerate once per (stmt, slot).
+ for slot in range(min(n_slots, MAX_SLOTS)):
+ slot_score = float(slot_lp[slot])
+ scored.append(
+ (
+ float(action_lp[ACTION_BIND_PLACEHOLDER]) + base + slot_score,
+ Edit(ACTION_BIND_PLACEHOLDER, stmt, slot=slot),
+ )
+ )
+ for payload in leaf_comps:
+ scored.append(
+ (
+ float(action_lp[ACTION_REPLACE_SUBTREE])
+ + base
+ + slot_score,
+ Edit(ACTION_REPLACE_SUBTREE, stmt, slot=slot,
+ payload=payload),
+ )
+ )
scored.append(
(float(action_lp[ACTION_REMOVE]) + base, Edit(ACTION_REMOVE, stmt))
)Note that fixing this changes recorded proposal counts, so the SLM-310 report JSON/Markdown needs regenerating.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for rest_idx in range(len(CONTAINER_RESTS)): | |
| scored.append( | |
| ( | |
| float(action_lp[ACTION_ADD_CONTAINER]) | |
| + base | |
| + float(comp_lp[comp]), | |
| Edit(ACTION_ADD_CONTAINER, stmt, comp, target=rest_idx), | |
| ) | |
| ) | |
| for slot in range(min(n_slots, MAX_SLOTS)): | |
| slot_score = float(slot_lp[slot]) | |
| scored.append( | |
| ( | |
| float(action_lp[ACTION_ADD]) | |
| + base | |
| + float(comp_lp[comp]) | |
| + float(slot_lp[slot]), | |
| + slot_score, | |
| Edit(ACTION_ADD, stmt, comp, slot), | |
| ) | |
| ) | |
| scored.append( | |
| ( | |
| float(action_lp[ACTION_BIND_PLACEHOLDER]) | |
| + base | |
| + slot_score, | |
| Edit(ACTION_BIND_PLACEHOLDER, stmt, slot=slot), | |
| ) | |
| ) | |
| for payload in leaf_comps: | |
| for rest_idx in range(len(CONTAINER_RESTS)): | |
| scored.append( | |
| ( | |
| float(action_lp[ACTION_INSERT_SUBTREE]) | |
| + base | |
| + float(comp_lp[comp]) | |
| + slot_score, | |
| Edit(ACTION_INSERT_SUBTREE, stmt, comp, slot, | |
| target=rest_idx, payload=payload), | |
| ) | |
| ) | |
| scored.append( | |
| ( | |
| float(action_lp[ACTION_REPLACE_SUBTREE]) | |
| + base | |
| + slot_score, | |
| Edit(ACTION_REPLACE_SUBTREE, stmt, slot=slot, | |
| payload=payload), | |
| ) | |
| ) | |
| scored.append( | |
| (float(action_lp[ACTION_REMOVE]) + base, Edit(ACTION_REMOVE, stmt)) | |
| ) | |
| scored.append( | |
| ( | |
| float(action_lp[ACTION_REMOVE_CONTAINER]) + base, | |
| Edit(ACTION_REMOVE_CONTAINER, stmt), | |
| ) | |
| ) | |
| for payload in range(len(V05_TEMPLATES)): | |
| scored.append( | |
| ( | |
| float(action_lp[ACTION_REPLACE_STATEMENT]) + base, | |
| Edit(ACTION_REPLACE_STATEMENT, stmt, payload=payload), | |
| ) | |
| ) | |
| for payload in range(len(V05_TEMPLATES)): | |
| scored.append( | |
| ( | |
| float(action_lp[ACTION_INSERT_STATEMENT]), | |
| Edit(ACTION_INSERT_STATEMENT, payload=payload), | |
| ) | |
| ) | |
| for rest_idx in range(len(CONTAINER_RESTS)): | |
| scored.append( | |
| ( | |
| float(action_lp[ACTION_ADD_CONTAINER]) | |
| base | |
| float(comp_lp[comp]), | |
| Edit(ACTION_ADD_CONTAINER, stmt, comp, target=rest_idx), | |
| ) | |
| ) | |
| for slot in range(min(n_slots, MAX_SLOTS)): | |
| slot_score = float(slot_lp[slot]) | |
| scored.append( | |
| ( | |
| float(action_lp[ACTION_ADD]) | |
| base | |
| float(comp_lp[comp]) | |
| slot_score, | |
| Edit(ACTION_ADD, stmt, comp, slot), | |
| ) | |
| ) | |
| for payload in leaf_comps: | |
| for rest_idx in range(len(CONTAINER_RESTS)): | |
| scored.append( | |
| ( | |
| float(action_lp[ACTION_INSERT_SUBTREE]) | |
| base | |
| float(comp_lp[comp]) | |
| slot_score, | |
| Edit(ACTION_INSERT_SUBTREE, stmt, comp, slot, | |
| target=rest_idx, payload=payload), | |
| ) | |
| ) | |
| # Component-independent proposals: enumerate once per (stmt, slot). | |
| for slot in range(min(n_slots, MAX_SLOTS)): | |
| slot_score = float(slot_lp[slot]) | |
| scored.append( | |
| ( | |
| float(action_lp[ACTION_BIND_PLACEHOLDER]) + base + slot_score, | |
| Edit(ACTION_BIND_PLACEHOLDER, stmt, slot=slot), | |
| ) | |
| ) | |
| for payload in leaf_comps: | |
| scored.append( | |
| ( | |
| float(action_lp[ACTION_REPLACE_SUBTREE]) | |
| base | |
| slot_score, | |
| Edit(ACTION_REPLACE_SUBTREE, stmt, slot=slot, | |
| payload=payload), | |
| ) | |
| ) | |
| scored.append( | |
| (float(action_lp[ACTION_REMOVE]) + base, Edit(ACTION_REMOVE, stmt)) | |
| ) | |
| scored.append( | |
| ( | |
| float(action_lp[ACTION_REMOVE_CONTAINER]) + base, | |
| Edit(ACTION_REMOVE_CONTAINER, stmt), | |
| ) | |
| ) | |
| for payload in range(len(V05_TEMPLATES)): | |
| scored.append( | |
| ( | |
| float(action_lp[ACTION_REPLACE_STATEMENT]) + base, | |
| Edit(ACTION_REPLACE_STATEMENT, stmt, payload=payload), | |
| ) | |
| ) | |
| for payload in range(len(V05_TEMPLATES)): | |
| scored.append( | |
| ( | |
| float(action_lp[ACTION_INSERT_STATEMENT]), | |
| Edit(ACTION_INSERT_STATEMENT, payload=payload), | |
| ) | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/slm_training/models/tree_edit_diffusion.py` around lines 1441 - 1512,
Hoist the component-independent BIND_PLACEHOLDER and REPLACE_SUBTREE proposal
generation out of the comp loop in the candidate enumeration function, while
preserving their per-slot and leaf_comps coverage and existing scores/Edit
construction. Keep component-dependent ADD, INSERT_SUBTREE, and container
proposals inside the comp loop, then regenerate the SLM-310 report JSON/Markdown
because proposal and audit counts will change.
| "harness.experiments.slm303_decode_budget_audit": { | ||
| "version": "v1", | ||
| "kind": "harness", | ||
| "paths": [ | ||
| "scripts/run_slm303_decode_budget_audit.py", | ||
| "tests/test_harnesses/model_build/test_decode_outcome.py", | ||
| "tests/test_scripts/test_run_slm303_decode_budget_audit.py", | ||
| "docs/design/iter-slm303-decode-budget-audit-20260724.json", | ||
| "docs/design/iter-slm303-decode-budget-audit-20260724.md" | ||
| ], | ||
| "history": [ | ||
| { | ||
| "version": "v1", | ||
| "date": "2026-07-24", | ||
| "note": "initial registration; SLM-303 decode-budget audit with hash pins, preregistered budget sweep, and taxonomy disposition" | ||
| } | ||
| ] | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether decode_outcome.py is claimed by any component and what the
# stamp verifier says about the changed files in this PR.
set -u
fd -t f 'versions.json' src/slm_training/resources || true
python - <<'PY'
import json, pathlib
reg = json.loads(pathlib.Path("src/slm_training/resources/versions.json").read_text())
targets = [
"src/slm_training/harnesses/model_build/decode_outcome.py",
"src/slm_training/harnesses/model_build/eval_runner.py",
"src/slm_training/evals/agentv.py",
"scripts/run_slm303_decode_budget_audit.py",
]
for target in targets:
owners = []
for cid, comp in reg["components"].items():
for p in comp.get("paths", []):
if p == target or (p.endswith("/") and target.startswith(p)):
owners.append((cid, p, comp.get("version")))
print(target, "->", owners or "UNCLAIMED")
PY
fd -t f 'verify_version_stamps.py' scripts --exec sed -n '1,40p'Repository: Tyler-R-Kendrick/slm-training
Length of output: 2595
Claim src/slm_training/harnesses/model_build/decode_outcome.py in the registry.
The decoder module is produced by this harness test component, but no component’s paths currently owns it while scripts/run_slm303_decode_budget_audit.py is already listed. Add the production module to prevent future changes from missing version stamps or stale-stamp signals.
🐛 Suggested claim
"paths": [
"scripts/run_slm303_decode_budget_audit.py",
+ "src/slm_training/harnesses/model_build/decode_outcome.py",
"tests/test_harnesses/model_build/test_decode_outcome.py",
"tests/test_scripts/test_run_slm303_decode_budget_audit.py",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "harness.experiments.slm303_decode_budget_audit": { | |
| "version": "v1", | |
| "kind": "harness", | |
| "paths": [ | |
| "scripts/run_slm303_decode_budget_audit.py", | |
| "tests/test_harnesses/model_build/test_decode_outcome.py", | |
| "tests/test_scripts/test_run_slm303_decode_budget_audit.py", | |
| "docs/design/iter-slm303-decode-budget-audit-20260724.json", | |
| "docs/design/iter-slm303-decode-budget-audit-20260724.md" | |
| ], | |
| "history": [ | |
| { | |
| "version": "v1", | |
| "date": "2026-07-24", | |
| "note": "initial registration; SLM-303 decode-budget audit with hash pins, preregistered budget sweep, and taxonomy disposition" | |
| } | |
| ] | |
| }, | |
| "harness.experiments.slm303_decode_budget_audit": { | |
| "version": "v1", | |
| "kind": "harness", | |
| "paths": [ | |
| "scripts/run_slm303_decode_budget_audit.py", | |
| "src/slm_training/harnesses/model_build/decode_outcome.py", | |
| "tests/test_harnesses/model_build/test_decode_outcome.py", | |
| "tests/test_scripts/test_run_slm303_decode_budget_audit.py", | |
| "docs/design/iter-slm303-decode-budget-audit-20260724.json", | |
| "docs/design/iter-slm303-decode-budget-audit-20260724.md" | |
| ], | |
| "history": [ | |
| { | |
| "version": "v1", | |
| "date": "2026-07-24", | |
| "note": "initial registration; SLM-303 decode-budget audit with hash pins, preregistered budget sweep, and taxonomy disposition" | |
| } | |
| ] | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/slm_training/resources/versions.json` around lines 4538 - 4555, Update
the paths list for harness.experiments.slm303_decode_budget_audit in the
versions registry to include
src/slm_training/harnesses/model_build/decode_outcome.py alongside the existing
script and test paths, so the production decoder module is covered by version
stamping and stale-change detection.
Source: Coding guidelines
| def __init__(self, trace: "RunTrace") -> None: | ||
| _load_local_env() | ||
| self.trace = trace | ||
| self.client: Any | None = None | ||
| self.error: str | None = None | ||
| api_key = _langsmith_api_key() | ||
| self.config = { | ||
| "enabled": bool(api_key) and _enabled("LANGSMITH_TRACING"), | ||
| "api_key_configured": bool(api_key), | ||
| "project": os.getenv("LANGSMITH_PROJECT", "slm-training"), | ||
| "endpoint": os.getenv("LANGSMITH_ENDPOINT") or None, | ||
| "workspace_id_configured": bool(os.getenv("LANGSMITH_WORKSPACE_ID")), | ||
| } | ||
| self.run_id = uuid.UUID(hex=trace.trace_id) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unguarded uuid.UUID(hex=trace.trace_id) can abort trace construction.
trace_id is rehydrated from a persisted run_dir/trace.json (str(existing.get("trace_id") or _hex_id(16))), so a legacy, truncated, or hand-edited value makes this raise ValueError inside RunTrace.__post_init__ — outside the try/except blocks that protect start/summary/finish. Every run reusing that run dir then fails at trace construction, which defeats the "observability must never stop a run" contract enforced everywhere else in this class.
Derive the run id defensively and disable export instead of propagating.
🛡️ Proposed fix
- self.run_id = uuid.UUID(hex=trace.trace_id)
+ try:
+ self.run_id = uuid.UUID(hex=trace.trace_id)
+ except ValueError:
+ # Non-W3C trace id (legacy/edited trace.json): keep local tracing
+ # working and skip the remote correlation instead of failing the run.
+ self.run_id = uuid.uuid4()
+ self.config["enabled"] = False
+ self.error = "ValueError"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def __init__(self, trace: "RunTrace") -> None: | |
| _load_local_env() | |
| self.trace = trace | |
| self.client: Any | None = None | |
| self.error: str | None = None | |
| api_key = _langsmith_api_key() | |
| self.config = { | |
| "enabled": bool(api_key) and _enabled("LANGSMITH_TRACING"), | |
| "api_key_configured": bool(api_key), | |
| "project": os.getenv("LANGSMITH_PROJECT", "slm-training"), | |
| "endpoint": os.getenv("LANGSMITH_ENDPOINT") or None, | |
| "workspace_id_configured": bool(os.getenv("LANGSMITH_WORKSPACE_ID")), | |
| } | |
| self.run_id = uuid.UUID(hex=trace.trace_id) | |
| def __init__(self, trace: "RunTrace") -> None: | |
| _load_local_env() | |
| self.trace = trace | |
| self.client: Any | None = None | |
| self.error: str | None = None | |
| api_key = _langsmith_api_key() | |
| self.config = { | |
| "enabled": bool(api_key) and _enabled("LANGSMITH_TRACING"), | |
| "api_key_configured": bool(api_key), | |
| "project": os.getenv("LANGSMITH_PROJECT", "slm-training"), | |
| "endpoint": os.getenv("LANGSMITH_ENDPOINT") or None, | |
| "workspace_id_configured": bool(os.getenv("LANGSMITH_WORKSPACE_ID")), | |
| } | |
| try: | |
| self.run_id = uuid.UUID(hex=trace.trace_id) | |
| except ValueError: | |
| # Non-W3C trace id (legacy/edited trace.json): keep local tracing | |
| # working and skip the remote correlation instead of failing the run. | |
| self.run_id = uuid.uuid4() | |
| self.config["enabled"] = False | |
| self.error = "ValueError" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/slm_training/runtime/telemetry/trace.py` around lines 76 - 89, Update
RunTrace.__init__ where self.run_id is derived from trace.trace_id so malformed
or truncated persisted IDs cannot propagate ValueError during construction.
Catch UUID parsing failures, set self.run_id to a safe non-exporting value, and
disable tracing/export through the existing configuration or error state while
preserving normal UUID behavior for valid IDs.
Tyler-R-Kendrick
left a comment
There was a problem hiding this comment.
Not merging — this branch is stale and conflicts semantically with main; a versions.json-only resolution is not possible.
Stack composition: this branch is a 7-commit stack (SLM-301 → 303 → 305 → 308 → 310 → 312). Main already contains SLM-301 (#835, 528feeb), SLM-303, and SLM-305 (a3dab82) in newer form, plus later work (SLM-425 tree-edit action-head/property head, ship-gates v4). The unique tail is only the three commits SLM-308/310/312.
Conflicts (13 files): a trial merge of 0dd8151 onto origin/main (eec4864) conflicts in: docs/design/agentv-evaluation.md, docs/design/langsmith-telemetry-smoke-20260724.json, scripts/run_slm299_reachability_audit.py, src/slm_training/evals/agentv.py, src/slm_training/harnesses/experiments/slm299_edit_reachability.py, src/slm_training/harnesses/model_build/eval_runner.py, src/slm_training/harnesses/model_build/ship_gates.py, src/slm_training/models/checkpoint_migrate.py, src/slm_training/models/tree_edit_diffusion.py, src/slm_training/resources/versions.json, src/slm_training/runtime/telemetry/trace.py, tests/test_models/test_tree_edit_diffusion.py, tests/test_runtime_trace.py.
Why this can't be auto-resolved:
- The PR's side of several conflicts is an older copy of what main now has — e.g. ship_gates.py would downgrade MEANINGFUL_METRIC_POLICY from openui_ship_gates_v4 back to v3/v2, checkpoint_migrate.py would drop the SLM-425 warm-start path, eval_runner.py would drop the temporal decode-trace evidence projection, trace.py would drop the dotenv fallback. Taking the PR side regresses main; taking main's side for all of them is correct but needs to be done deliberately.
- The SLM-308/310/312 tail commits themselves modify tree_edit_diffusion.py (ACTION_NAMES, INVERSE_TO_MUTATION_KINDS, REASON_* codes, apply() reason out-param) built on the pre-SLM-425 N_ACTIONS=11 file. Main has since rewritten that file (property-value head, grown action_head), so the SLM-310 additions need a real semantic rebase, not a textual one.
Suggested path: rebase the branch onto current main keeping only the SLM-308/310/312 commits (drop the already-merged 301/303/305 commits), port the tree_edit_diffusion.py additions onto the SLM-425 action space, re-run tests/test_harnesses/experiments/test_slm30{8,10,12}*.py plus verify_version_stamps, and re-push. The SLM-308/310/312 work is not duplicated on main, so the content is still wanted.
|
Closing as superseded — content already landed on main via other PRs (see review comments for superseding commits). Reopen only with a rebased, still-unique delta. |
Summary
SLM-312 / LAR2-04: trains on seedward and on-policy valid states using the existing supervision scaffolds.
gold_only(existing corruption chain),seedward(offline oracle-guided walk from the decode seed toward gold — every intermediate valid and strictly distance-decreasing via the SLM-308 oracle),on_policy(immutable, content-addressed beam-trajectory snapshots incl. high-value wrong states, verifier failures, abstentions). Every row: explicit source + provenance {parent checkpoint, source commit, rollout config, prompt/suite hashes, gold-visibility policy}; sha256 rows + tamper-evident manifest; fail-closed leakage guards (train↔held-out AST-fingerprint overlap).Verification
harness.experiments.slm312_state_sourcesv1); repo_policy ok; git diff --check clean; ruff + format clean.outputs/slm312/state_snapshot.jsonl+ manifest (43 rows, 37 unique states, 1 leak collision dropped-and-counted).Closes SLM-312.
Summary by CodeRabbit
New Features
Documentation
Tests