VSS2-03: Torch-free topology-to-finite-domain adapter (SLM-67)#326
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR adds dependency-closed verification capsule graphs for ChangesVerification capsule graphs
Finite-domain topology solver
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ProgramSpec
participant CapsuleGraphDeriver
participant TypedAST
participant TarjanSCC
ProgramSpec->>CapsuleGraphDeriver: provide specification
CapsuleGraphDeriver->>TypedAST: walk scopes and references
TypedAST-->>CapsuleGraphDeriver: dependency edges
CapsuleGraphDeriver->>TarjanSCC: compute reference components
TarjanSCC-->>CapsuleGraphDeriver: verification capsules
sequenceDiagram
participant TopologyTree
participant TopologyAdapter
participant GrammarCodec
participant FiniteDomainState
TopologyTree->>TopologyAdapter: provide bounded active nodes
TopologyAdapter->>GrammarCodec: query legal productions
GrammarCodec-->>TopologyAdapter: production identifiers
TopologyAdapter->>FiniteDomainState: encode topology edit domains
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
src/slm_training/data/progspec/capsules.py (1)
13-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid publishing dependency kinds that V1 never emits.
DEFINES,CONTAINMENT, andEFFECTare exported API values, but the implementation and design caveats defer their semantics. Remove them until implemented to avoid cementing an unsupported contract.As per coding guidelines, “Do not add abstractions, dependencies, boilerplate, or files unless they are needed.”
🤖 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/data/progspec/capsules.py` around lines 13 - 21, Update the DependencyKind enum to remove the unsupported DEFINES, CONTAINMENT, and EFFECT values, retaining only dependency kinds emitted and supported by V1.Source: Coding guidelines
🤖 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.
Inline comments:
In `@src/slm_training/data/progspec/capsules.py`:
- Around line 316-337: The capsule construction around _tarjan_sccs must account
for inter-SCC reference edges: for each capsule, compute and include the
transitive reachable dependency nodes in node_ids, or explicitly represent those
capsule dependencies instead of treating node_ids as closed. Preserve
deterministic ordering and ensure external_dependencies and entry_node_id remain
consistent with the resulting closure.
- Around line 174-199: Preserve the existing path_to_node mapping for the root
statement when creating the synthetic root, so traversal and member/external
assignment still resolve to that statement node. Update the ROOT_OUTPUT
association to target the root statement explicitly rather than checking
definitions, since derive_scope_contracts excludes “root” from definitions. Keep
the synthetic root node available without replacing the root statement mapping.
- Around line 174-187: Change the statement traversal around definition_to_node
and _walk so declarations become resolvable only after their statement has been
visited, causing references to later statements to fail validation. Apply the
same ordering fix to the corresponding logic noted at the additional location,
while preserving resolution of previously declared names.
- Around line 36-58: Update ScopeNode.from_dict and the related persistence
deserialization entry points to validate serialized graph data before
constructing immutable objects: enforce supported version values, required field
types, valid AST/member paths, unique node identifiers, existing edge endpoints,
and capsule membership. Remove permissive str(...) coercions and reject
malformed or unknown data with a clear validation error rather than silently
normalizing it.
In `@src/slm_training/dsl/solver/__init__.py`:
- Around line 5-25: Update the solver package exports in __init__.py to include
the documented public contracts SupportVerdict and TopologyNodeLike, importing
them from their defining module and adding them to __all__. If they are
intentionally submodule-only instead, update the documentation accordingly, but
preserve the documented API contract.
In `@src/slm_training/dsl/solver/state.py`:
- Around line 74-84: Make the state represented by the dataclass containing
__post_init__ and to_dict genuinely immutable by defensively freezing or
recursively copying all JSON-compatible dictionary data, including nested
values, before fingerprint computation. Ensure post-construction mutations
cannot alter the fingerprinted state, and have to_dict return a detached
representation rather than exposing metadata directly. Preserve the existing
serialization shape and uniqueness validation.
In `@src/slm_training/dsl/solver/topology_adapter.py`:
- Around line 212-243: Update the finite edit enumeration in the topology
adapter to emit TopologyAction.CONTRACT alongside KEEP, DELETE, and STOP when
the node supports it, preserving the existing production, arity, and slot
fields. Update the regression test to assert exact action coverage and the
expected four structural candidates, including CONTRACT.
- Around line 150-152: Update the fallback handling in the result-building
method so it never appends mask_id when codec.unk_id is absent. Only append
unk_id after validating that it is a non-special ID present in id_to_production;
otherwise leave result empty or return the empty result.
- Around line 60-68: Update TopologyEdit.from_value to validate that value has
the expected topology-edit DomainValue tag and exactly four payload items before
unpacking value.payload. Reject invalid tags or payload lengths using the
module’s established validation/error convention, while preserving the existing
decoding for valid values.
In `@tests/test_dsl/test_topology_adapter.py`:
- Around line 135-144: Update test_importing_adapter_does_not_import_torch to
execute the import and Torch-presence assertion in a fresh subprocess rather
than the current interpreter. Start the subprocess with clean module state,
import slm_training.dsl.solver.topology_adapter there, and assert that torch is
absent without relying on the parent process’s sys.modules.
---
Nitpick comments:
In `@src/slm_training/data/progspec/capsules.py`:
- Around line 13-21: Update the DependencyKind enum to remove the unsupported
DEFINES, CONTAINMENT, and EFFECT values, retaining only dependency kinds emitted
and supported by V1.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ad5b7e34-52d2-4ec9-9583-345af49ca60e
📒 Files selected for processing (9)
docs/design/vss2-01-verification-capsules.mddocs/design/vss2-03-topology-edits.mdsrc/slm_training/data/progspec/__init__.pysrc/slm_training/data/progspec/capsules.pysrc/slm_training/dsl/solver/__init__.pysrc/slm_training/dsl/solver/state.pysrc/slm_training/dsl/solver/topology_adapter.pytests/test_data/test_progspec.pytests/test_dsl/test_topology_adapter.py
| def to_dict(self) -> dict[str, Any]: | ||
| data = asdict(self) | ||
| data["ast_path"] = list(self.ast_path) | ||
| data["member_paths"] = [list(p) for p in self.member_paths] | ||
| data["definitions"] = list(self.definitions) | ||
| data["external_dependencies"] = list(self.external_dependencies) | ||
| return data | ||
|
|
||
| @classmethod | ||
| def from_dict(cls, data: Mapping[str, Any]) -> ScopeNode: | ||
| return cls( | ||
| node_id=str(data["node_id"]), | ||
| scope_id=str(data["scope_id"]) if data.get("scope_id") else None, | ||
| kind=str(data["kind"]), | ||
| ast_path=tuple(data.get("ast_path") or ()), | ||
| member_paths=tuple( | ||
| tuple(p) for p in (data.get("member_paths") or ()) | ||
| ), | ||
| definitions=tuple(str(v) for v in data.get("definitions") or ()), | ||
| external_dependencies=tuple( | ||
| str(v) for v in data.get("external_dependencies") or () | ||
| ), | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Validate serialized graph data instead of coercing malformed input.
These persistence entry points silently coerce arbitrary values with str(...) and accept unsupported versions, unknown endpoints, and malformed paths. Validate types, version, node uniqueness, edge endpoints, and capsule membership before constructing the immutable graph.
As per coding guidelines, “Always perform input validation at trust boundaries.”
Also applies to: 78-85, 105-114, 140-151
🤖 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/data/progspec/capsules.py` around lines 36 - 58, Update
ScopeNode.from_dict and the related persistence deserialization entry points to
validate serialized graph data before constructing immutable objects: enforce
supported version values, required field types, valid AST/member paths, unique
node identifiers, existing edge endpoints, and capsule membership. Remove
permissive str(...) coercions and reject malformed or unknown data with a clear
validation error rather than silently normalizing it.
Source: Coding guidelines
| for contract in statements: | ||
| node_id = f"{contract.scope_id}:node" | ||
| nodes[node_id] = ScopeNode( | ||
| node_id=node_id, | ||
| scope_id=contract.scope_id, | ||
| kind="statement", | ||
| ast_path=contract.ast_path, | ||
| member_paths=(), | ||
| definitions=contract.definitions, | ||
| external_dependencies=(), | ||
| ) | ||
| path_to_node[contract.ast_path] = node_id | ||
| for name in contract.definitions: | ||
| definition_to_node[name] = node_id |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Forward references are currently accepted.
definition_to_node is populated with every declaration before _walk, so a reference to a later statement resolves successfully. The added validation only rejects undefined binders. Either track declarations in traversal order or rename the documented contract to undefined-reference validation.
Also applies to: 251-268
🤖 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/data/progspec/capsules.py` around lines 174 - 187, Change
the statement traversal around definition_to_node and _walk so declarations
become resolvable only after their statement has been visited, causing
references to later statements to fail validation. Apply the same ordering fix
to the corresponding logic noted at the additional location, while preserving
resolution of previously declared names.
| for contract in statements: | ||
| node_id = f"{contract.scope_id}:node" | ||
| nodes[node_id] = ScopeNode( | ||
| node_id=node_id, | ||
| scope_id=contract.scope_id, | ||
| kind="statement", | ||
| ast_path=contract.ast_path, | ||
| member_paths=(), | ||
| definitions=contract.definitions, | ||
| external_dependencies=(), | ||
| ) | ||
| path_to_node[contract.ast_path] = node_id | ||
| for name in contract.definitions: | ||
| definition_to_node[name] = node_id | ||
|
|
||
| # Synthetic root node. | ||
| nodes[root_id] = ScopeNode( | ||
| node_id=root_id, | ||
| scope_id=None, | ||
| kind="root", | ||
| ast_path=(), | ||
| member_paths=(), | ||
| definitions=(), | ||
| external_dependencies=(), | ||
| ) | ||
| path_to_node[()] = root_id |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve the root statement mapping and connect ROOT_OUTPUT to it.
The synthetic root overwrites path_to_node[()], orphaning the root statement from traversal and member/external assignment. Additionally, derive_scope_contracts excludes "root" from definitions, so the current ROOT_OUTPUT condition can never succeed.
Proposed fix
for contract in statements:
...
path_to_node[contract.ast_path] = node_id
...
+ root_statement_id = path_to_node.get(())
+
nodes[root_id] = ScopeNode(
...
)
- path_to_node[()] = root_id
+ path_to_node.setdefault((), root_id)
- if "root" in definition_to_node:
+ if root_statement_id is not None:
edges.append(
ScopeEdge(
source=root_id,
- target=definition_to_node["root"],
+ target=root_statement_id,
kind=DependencyKind.ROOT_OUTPUT,
role="root",
)
)Also applies to: 302-311
🤖 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/data/progspec/capsules.py` around lines 174 - 199, Preserve
the existing path_to_node mapping for the root statement when creating the
synthetic root, so traversal and member/external assignment still resolve to
that statement node. Update the ROOT_OUTPUT association to target the root
statement explicitly rather than checking definitions, since
derive_scope_contracts excludes “root” from definitions. Keep the synthetic root
node available without replacing the root statement mapping.
| # Compute SCCs over statement nodes using reference edges. | ||
| sccs = _tarjan_sccs({n.node_id: n for n in node_list}, edge_list) | ||
|
|
||
| capsules: list[VerificationCapsule] = [] | ||
| for index, component in enumerate(sccs): | ||
| node_ids = tuple(sorted(component)) | ||
| entry = node_ids[0] | ||
| external = sorted( | ||
| { | ||
| dep | ||
| for nid in node_ids | ||
| for dep in nodes[nid].external_dependencies | ||
| } | ||
| ) | ||
| capsules.append( | ||
| VerificationCapsule( | ||
| capsule_id=f"{spec.id}:capsule:{index}", | ||
| node_ids=node_ids, | ||
| entry_node_id=entry, | ||
| external_dependencies=tuple(external), | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
SCC membership does not make capsules dependency-closed.
For an edge A → B where the nodes are not cyclic, Tarjan returns separate capsules and A’s capsule omits B. Compute each SCC’s reachable dependency closure, or explicitly model dependencies between capsules instead of claiming closed node_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/data/progspec/capsules.py` around lines 316 - 337, The
capsule construction around _tarjan_sccs must account for inter-SCC reference
edges: for each capsule, compute and include the transitive reachable dependency
nodes in node_ids, or explicitly represent those capsule dependencies instead of
treating node_ids as closed. Preserve deterministic ordering and ensure
external_dependencies and entry_node_id remain consistent with the resulting
closure.
| from slm_training.dsl.solver.state import DomainValue, FiniteDomainState, HoleDomain, HoleId | ||
| from slm_training.dsl.solver.topology_adapter import ( | ||
| TopologyAction, | ||
| TopologyAdapterConfig, | ||
| TopologyEdit, | ||
| TopologyHole, | ||
| derive_topology_holes, | ||
| legal_topology_productions, | ||
| ) | ||
|
|
||
| __all__ = [ | ||
| "DomainValue", | ||
| "FiniteDomainState", | ||
| "HoleDomain", | ||
| "HoleId", | ||
| "TopologyAction", | ||
| "TopologyAdapterConfig", | ||
| "TopologyEdit", | ||
| "TopologyHole", | ||
| "derive_topology_holes", | ||
| "legal_topology_productions", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Export the remaining documented public contracts.
SupportVerdict and TopologyNodeLike are listed as added contracts in the design document but are omitted from the package API. Export them here or explicitly document them as submodule-only.
🤖 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/dsl/solver/__init__.py` around lines 5 - 25, Update the
solver package exports in __init__.py to include the documented public contracts
SupportVerdict and TopologyNodeLike, importing them from their defining module
and adding them to __all__. If they are intentionally submodule-only instead,
update the documentation accordingly, but preserve the documented API contract.
| @classmethod | ||
| def from_value(cls, value: DomainValue) -> TopologyEdit: | ||
| action_name, production_id, arity, slot_id = value.payload | ||
| return cls( | ||
| action=TopologyAction[action_name], | ||
| production_id=int(production_id), | ||
| arity=int(arity), | ||
| slot_id=int(slot_id), | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Validate the tagged value before decoding it.
A different DomainValue tag with a four-item payload is silently accepted as a topology edit. Reject incorrect tags and malformed payload lengths before unpacking.
Proposed fix
`@classmethod`
def from_value(cls, value: DomainValue) -> TopologyEdit:
+ if value.tag != "topology_edit":
+ raise ValueError(f"expected topology_edit, got {value.tag!r}")
+ if len(value.payload) != 4:
+ raise ValueError("topology_edit payload must contain four fields")
action_name, production_id, arity, slot_id = value.payloadAs per coding guidelines, “Always perform input validation at trust boundaries.”
📝 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.
| @classmethod | |
| def from_value(cls, value: DomainValue) -> TopologyEdit: | |
| action_name, production_id, arity, slot_id = value.payload | |
| return cls( | |
| action=TopologyAction[action_name], | |
| production_id=int(production_id), | |
| arity=int(arity), | |
| slot_id=int(slot_id), | |
| ) | |
| `@classmethod` | |
| def from_value(cls, value: DomainValue) -> TopologyEdit: | |
| if value.tag != "topology_edit": | |
| raise ValueError(f"expected topology_edit, got {value.tag!r}") | |
| if len(value.payload) != 4: | |
| raise ValueError("topology_edit payload must contain four fields") | |
| action_name, production_id, arity, slot_id = value.payload | |
| return cls( | |
| action=TopologyAction[action_name], | |
| production_id=int(production_id), | |
| arity=int(arity), | |
| slot_id=int(slot_id), | |
| ) |
🤖 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/dsl/solver/topology_adapter.py` around lines 60 - 68, Update
TopologyEdit.from_value to validate that value has the expected topology-edit
DomainValue tag and exactly four payload items before unpacking value.payload.
Reject invalid tags or payload lengths using the module’s established
validation/error convention, while preserving the existing decoding for valid
values.
Source: Coding guidelines
| if not result: | ||
| unk_id = getattr(codec, "unk_id", mask_id) | ||
| result.append(unk_id) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not return an explicitly illegal fallback production.
When unk_id is absent, this appends mask_id, even though Line 129 excludes that ID as special. Return an empty result or only append a validated, non-special unk_id present in id_to_production.
🤖 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/dsl/solver/topology_adapter.py` around lines 150 - 152,
Update the fallback handling in the result-building method so it never appends
mask_id when codec.unk_id is absent. Only append unk_id after validating that it
is a non-special ID present in id_to_production; otherwise leave result empty or
return the empty result.
| # Complete edit tuples for structural actions. | ||
| for action in (TopologyAction.KEEP, TopologyAction.DELETE, TopologyAction.STOP): | ||
| values.append( | ||
| TopologyEdit( | ||
| action=action, | ||
| production_id=node.production_id, | ||
| arity=len(node.children), | ||
| slot_id=node.slot_id, | ||
| ).to_value() | ||
| ) | ||
|
|
||
| # EXPAND actions: enumerate legal complete edits (production, arity, slot). | ||
| if node.node_type != "leaf": | ||
| legal_pids = legal_topology_productions(codec, node.node_type) | ||
| max_arity = min(config.topology_max_arity, 8) | ||
| for pid in legal_pids: | ||
| token = codec.id_to_production.get(pid, "") | ||
| is_container = token.startswith("+") or token in { | ||
| "[", | ||
| FRAGMENT_CHUNK, | ||
| } | ||
| arity_range = range(max_arity + 1) if is_container else range(1) | ||
| for arity in arity_range: | ||
| for slot_id in ([0, *range(1, len(slots) + 1)]) if slots else [0]: | ||
| values.append( | ||
| TopologyEdit( | ||
| action=TopologyAction.EXPAND, | ||
| production_id=pid, | ||
| arity=arity, | ||
| slot_id=slot_id, | ||
| ).to_value() | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Include CONTRACT in the finite edit domain.
TopologyAction.CONTRACT is public but never emitted, so derived holes cannot represent that action. The test’s “4 structural” bound also indicates four structural candidates were intended. Enumerate CONTRACT where legal and assert exact action coverage in the regression test.
🤖 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/dsl/solver/topology_adapter.py` around lines 212 - 243,
Update the finite edit enumeration in the topology adapter to emit
TopologyAction.CONTRACT alongside KEEP, DELETE, and STOP when the node supports
it, preserving the existing production, arity, and slot fields. Update the
regression test to assert exact action coverage and the expected four structural
candidates, including CONTRACT.
| def test_importing_adapter_does_not_import_torch(): | ||
| import importlib | ||
| import sys | ||
|
|
||
| # Snapshot current torch presence, then fresh-import the adapter module. | ||
| had_torch = "torch" in sys.modules | ||
| if "slm_training.dsl.solver.topology_adapter" in sys.modules: | ||
| del sys.modules["slm_training.dsl.solver.topology_adapter"] | ||
| importlib.import_module("slm_training.dsl.solver.topology_adapter") | ||
| assert ("torch" in sys.modules) == had_torch |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Run the Torch-free assertion in a fresh interpreter.
The adapter is already imported through Lines 6–12. If that initial import loads Torch, had_torch is True and this test still passes. Use a subprocess so the assertion begins with clean module state.
Proposed fix
def test_importing_adapter_does_not_import_torch():
- import importlib
+ import subprocess
import sys
- # Snapshot current torch presence, then fresh-import the adapter module.
- had_torch = "torch" in sys.modules
- if "slm_training.dsl.solver.topology_adapter" in sys.modules:
- del sys.modules["slm_training.dsl.solver.topology_adapter"]
- importlib.import_module("slm_training.dsl.solver.topology_adapter")
- assert ("torch" in sys.modules) == had_torch
+ code = """
+import sys
+assert "torch" not in sys.modules
+import slm_training.dsl.solver.topology_adapter
+assert "torch" not in sys.modules
+"""
+ subprocess.run([sys.executable, "-c", code], check=True)📝 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 test_importing_adapter_does_not_import_torch(): | |
| import importlib | |
| import sys | |
| # Snapshot current torch presence, then fresh-import the adapter module. | |
| had_torch = "torch" in sys.modules | |
| if "slm_training.dsl.solver.topology_adapter" in sys.modules: | |
| del sys.modules["slm_training.dsl.solver.topology_adapter"] | |
| importlib.import_module("slm_training.dsl.solver.topology_adapter") | |
| assert ("torch" in sys.modules) == had_torch | |
| def test_importing_adapter_does_not_import_torch(): | |
| import subprocess | |
| import sys | |
| code = """ | |
| import sys | |
| assert "torch" not in sys.modules | |
| import slm_training.dsl.solver.topology_adapter | |
| assert "torch" not in sys.modules | |
| """ | |
| subprocess.run([sys.executable, "-c", code], check=True) |
🤖 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_dsl/test_topology_adapter.py` around lines 135 - 144, Update
test_importing_adapter_does_not_import_torch to execute the import and
Torch-presence assertion in a fresh subprocess rather than the current
interpreter. Start the subprocess with clean module state, import
slm_training.dsl.solver.topology_adapter there, and assert that torch is absent
without relying on the parent process’s sys.modules.
The fleet independently shipped SLM-59 (Torch-free finite-domain support lattice) to main via #321/#326. This branch had its own SLM-59 implementation, now redundant. Per "do not reimplement what already exists," resolve the add/add collision by taking main's canonical dsl/solver/ (state.py, adapters.py, __init__.py) and test_solver_state.py verbatim, dropping this branch's duplicate. #322 collapses to its unique, solver-independent LDI contribution (DecisionEventV2 + diagnostics, causal capture, removable adapter). Verified: main's solver test + all LDI suites pass together (141), repo_policy ok. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U5Fy8PnMTV54p675y9T3B8
VSS2-03: adapts GrammarDiffusionModel's bounded production-tree topology nodes into the canonical finite-domain solver state (main's DomainValue tag/payload_json API), so each active topology node becomes a stable semantic hole whose values are complete edit tuples. Carrier + hard-domain enumeration only; does not alter model generation. Torch-free; verified locally (topology + existing solver tests pass). No ship claim.
b7d75b2 to
fb92790
Compare
2 participants
Implements SLM-67.
Changes
src/slm_training/dsl/solver/package:state.py— immutableHoleId,DomainValue,HoleDomain,FiniteDomainState,SupportVerdict.topology_adapter.py— Torch-freeTopologyAction,TopologyEdit,TopologyAdapterConfig,legal_topology_productions, andderive_topology_holes.__init__.pyexporting public contracts.(action, production_id, arity, slot_id)for each active topology node instead of independent logits.torchorgrammar_diffusion.py; callers passTopologyNodeinstances into the Torch-freeTopologyNodeLikeprotocol.tests/test_dsl/test_topology_adapter.pywith regressions for completeness, finiteness, uniqueness, round-trip, fingerprint stability, and torch-free import.docs/design/vss2-03-topology-edits.md.Verified
ruff checkpasses.python -m compileallpasses.pytest tests/test_dsl/test_topology_adapter.pypasses (6 tests).python -m scripts.repo_policyok.git diff --checkclean.Honest caveats
VerificationCapsule) is scaffolded through stableHoleIdpaths and is a follow-up.Summary by CodeRabbit
New Features
Documentation
Tests