Skip to content

LTM layout: add integration/unit coverage for detected.truncated fallback branch #467

@bpowers

Description

@bpowers

Problem

The if detected.truncated { return None; } fallback branch in try_detect_ltm_loops_incremental (at src/simlin-engine/src/layout/mod.rs:3838) no longer has any dedicated test coverage.

This branch is part of the documented fallback chain: when model_detected_loops returns truncated = true, try_detect_ltm_loops_incremental returns None so the caller (compute_metadata) falls back to persisted loop_metadata instead of silently treating the model as structurally acyclic. This matters in production: WRLD3-scale models routinely exceed the structural-detection cap.

How it was lost

Introduced in PR #461 (branch ltm-perf-enable-always), iteration 16: we split the LTM structural-detection cap from the LTM-synthesis cap by raising MAX_LTM_DETECTED_LOOPS to 100_000 while keeping the synthesis cap MAX_LTM_TOTAL_CIRCUITS at 10_000.

Before the split, an integration test in src/simlin-engine/tests/layout.rs called test_compute_metadata_falls_back_on_truncated_loop_detection built 10_001 disjoint 3-cycles to trip the 10_000 synthesis cap and verify that compute_metadata fell back to persisted loop_metadata on truncated = true.

With the new 100k structural cap, a direct reproduction would need ~100k disjoint cycles (300k+ variables), which pushes CI beyond a reasonable wall time. We tried a complete-bipartite K(N,N) shape to multiply the distinct-cycle count by 2^N - 1, but Johnson's DFS still traverses roughly N!/e raw cycles before the per-SCC sorted-node-set dedup drops them, which is infeasible at N >= 14.

So the integration test was deleted. The sibling fallback branch (on simulation failure) is still covered by test_ltm_fallback_on_sim_error in src/simlin-engine/src/layout/layout_tests.rs:1542, but the truncated branch has no regression guard.

Why it matters

  • The fallback is the documented behavior for dense real-world graphs; silent regression would make compute_metadata emit empty feedback-loop metadata instead of the persisted loop_metadata.
  • The branch is the only thing distinguishing "empty loops because there are no cycles" (return Some(vec![])) from "empty loops because we gave up enumerating" (return None). Conflating them is a real risk during future refactors of either the cap constants or the DetectedLoopsResult shape.

Component(s) affected

  • src/simlin-engine/src/layout/mod.rs -- try_detect_ltm_loops_incremental
  • src/simlin-engine/tests/layout.rs -- integration test surface
  • src/simlin-engine/src/layout/layout_tests.rs -- sibling unit-test surface

Possible approaches

Option (a) -- preferred: Refactor try_detect_ltm_loops_incremental so DetectedLoopsResult is injectable. This enables a direct unit test of the truncated-branch dispatch that is fast, deterministic, and does not require fabricating pathological graph topologies. Likely shape: split the current function into a thin shell that calls model_detected_loops and a pure inner function that takes the result by value, then unit-test the inner function with a hand-crafted DetectedLoopsResult { truncated: true, loops: vec![] }.

Option (b) -- fallback: Wire a dedicated WRLD3-scale integration test (gated behind an expensive-tests feature) that actually trips MAX_LTM_DETECTED_LOOPS during structural enumeration. Slower, non-deterministic across CI hardware, and harder to diagnose when it breaks, but exercises the full salsa pipeline end-to-end.

Context

Identified during PR #461 review while closing out iteration 16. The deletion was load-bearing for keeping CI fast but left a coverage gap we should not silently absorb.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions