feat(grammar-diffusion): topology solver capsule fallback + unit test (SLM-71 follow-up)#371
Conversation
- Verify closure runs over a tiny ProductionCodec tree and survivors are a subset of the original complete edit domain.
…lver - Detect topology_capsule_solver=True and probe the active DSL pack for capsule slots (builder, summary extractor, materializer, global oracle). - When slots are unavailable, fall back to whole-tree exact closure and record a capsule_fallback trace entry instead of silently weakening. - Add regression test for the fallback behavior.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe topology solver now probes optional capsule pack slots and records capsule availability or fallback state in its trace. New regression tests verify monotone topology pruning and fallback behavior when capsule slots are unavailable. ChangesTopology solver updates
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
Follow-up to #356 (SLM-71). What changed
Verification
CaveatsFull capsule-plan/SCC integration, reversible backtracking, and model/energy ranker ordering remain future work. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/slm_training/models/grammar_diffusion.py (1)
1637-1676: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix dead code and lost fallback telemetry.
The
tracedictionary assigned here is unconditionally overwritten by the assignment on line 1663. As a result, the "honest gap" telemetry is lost when capsules are available but not yet implemented in the model. Additionally, the comment implies the model hooks are missing, not the pack slots (sincecapsule_availableis true).Consolidate the assignments to preserve the
capsule_fallbackfield.💡 Proposed fix
if use_capsules and capsule_available: # VSS3-03 future work: drive solve_capsule_graph with topology - # capsule hooks. Until those pack slots are implemented, fall + # capsule hooks. Until those hooks are implemented, fall # back to whole-tree exact closure and record the honest gap. - trace = { - "enabled": True, - "capsule_mode": True, - "capsule_fallback": "pack_capsule_slots_unimplemented", - } # Fall through to the whole-tree prune below. survivors, result = topology_solver_prune( root, self.codec, adapter_config, slot_inventory, output_kind, bounds, max_queries=max( 1, min( self.config.topology_max_active * 4, self.config.topology_solver_max_verifier_calls or 64, ), ), ) trace = { "enabled": True, "capsule_mode": use_capsules, "capsule_available": capsule_available, "reached_fixed_point": result.reached_fixed_point, "stop_reason": result.stop_reason, "candidates_removed": result.counters.candidates_removed, "support_queries": result.counters.support_queries, "verifier_calls": result.counters.verifier_calls, "survivor_count": len(survivors), } if use_capsules and not capsule_available: trace["capsule_fallback"] = "pack_capsule_slots_unimplemented" + elif use_capsules and capsule_available: + trace["capsule_fallback"] = "model_capsule_hooks_unimplemented"🤖 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/grammar_diffusion.py` around lines 1637 - 1676, Consolidate the trace construction in the topology-pruning flow so the initial capsule fallback telemetry is not overwritten by the later assignment. Update the fallback condition and message to reflect missing model hooks when use_capsules and capsule_available are true, while retaining the existing unavailable-capsule fallback behavior and all solver metrics.
🤖 Prompt for all review comments with 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.
Outside diff comments:
In `@src/slm_training/models/grammar_diffusion.py`:
- Around line 1637-1676: Consolidate the trace construction in the
topology-pruning flow so the initial capsule fallback telemetry is not
overwritten by the later assignment. Update the fallback condition and message
to reflect missing model hooks when use_capsules and capsule_available are true,
while retaining the existing unavailable-capsule fallback behavior and all
solver metrics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 388ba0d3-5101-4e86-b10a-8f1d201a7327
📒 Files selected for processing (3)
src/slm_training/models/grammar_diffusion.pytests/test_dsl/test_topology_solver.pytests/test_models/test_grammar_diffusion_solver.py
…llback When topology_capsule_solver=True and the DSL pack exposes capsule slots, call topology_capsule_solver_solve and convert solved capsule decisions into survivor edits. If the coordinator returns non-solved, empty survivors, or raises, fall back to whole-tree topology_solver_prune with an honest trace entry. Keeps the existing pack-slot-missing fallback unchanged.
"Follow-up to #356 (SLM-71).\n\n## What changed\n- Added
test_topology_solver_prune_runs_and_is_monotone— direct unit test thattopology_solver_pruneruns exact closure over a smallSolverTopologyNodetree and survivors are a subset of the original complete edit domain.\n- Added a synthetic joint-capsule coordinator intopology_solver.py:\n -_derive_synthetic_capsule_graphbuilds one jointCapsuleGraphover all active topology nodes.\n -TopologyCapsuleProblemBuilder,TopologyCapsuleSummaryExtractor,TopologyCapsuleMaterializer,TopologyCapsuleTerminalChecker, andTopologyCapsuleGlobalVerifierwire the capsule solver protocol.\n -topology_capsule_solver_solveexposes the coordinator and returns(assembled_source, CapsuleSolveResult).\n- Wired the capsule solver intoGrammarDiffusionModel._topology_solver_survivorswhentopology_capsule_solver=Trueand the active DSL pack exposes the required capsule slots. If the coordinator does not return a solved result with non-empty survivors, or if it raises, the model falls back to whole-treetopology_solver_prunewith an honestcapsule_fallbacktrace entry. If the pack slots are missing, the existingpack_capsule_slots_unimplementedfallback remains.\n- Addedtest_topology_capsule_solver_runs_without_errorand kepttest_capsule_mode_falls_back_when_pack_slots_missingto document the honest gap.\n\n## Verification\n-python -m pytest tests/test_dsl tests/test_harnesses/model_build tests/test_models -q— 838 passed, 5 skipped, 15 deselected\n-.githooks/check-changed— passed\n-python -m scripts.repo_policy— ok\n-git diff --check— clean\n\n## Caveats\nThe synthetic capsule graph is a wiring fixture: it exercisessolve_capsule_graphand SCC-joint plumbing but does not yet derive capsules from a realProgramSpec. Full pack capsule slots, reversible backtracking, and model/energy ranker ordering remain future work.\n"