Skip to content

Commit a555fd1

Browse files
authored
doc: align version-guard, loop-helper, and reclassify docs with the code (#1013)
Follow-up to #1012, addressing the three codex review findings that arrived as it merged. All three are the stale-claim class: documentation describing a state of the world the code no longer has. - **`simlin-mcp` version guard**: the comment and skip message described CI as a tagless shallow clone where the guard "is a no-op" — the opposite of what `ci.yaml`'s `fetch-tags: true` checkout (added in #1012) does. An unexpected skip in CI would have been misread as documented-normal behavior. Both now state that CI fetches tags specifically so the guard runs, and that a skip is expected only on a local clone without them. (Verified live: with the fresh `pysimlin-v0.8.0` tag fetched, the guard passes against the 0.8.0 release.) - **Engine module map**: `src/ltm/` still advertised the deleted top-level `detect_loops` helper; anyone following the documented surface would import a nonexistent API. - **`reclassify_loops_from_results` rustdoc**: claimed pysimlin `Run.loops` "still reclassifies via its own Python `LoopPolarity.from_runtime_scores` mirror (slot-0 only)" and "exposes the all-slots engine path separately as `Run.loops_runtime`" — both false: `Run.loops` rides this helper via `Sim.get_loops_runtime`, there is no separate property, and the Python classifier no longer exists. The A2A-semantics note is now correctly a two-site comparison (this helper's all-slots concatenation vs discovery's strongest-path scalar) instead of three. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01S3vQ72pUeYXdMgCv7sSjaB
1 parent c7ba043 commit a555fd1

3 files changed

Lines changed: 25 additions & 33 deletions

File tree

src/simlin-engine/CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ Unit checking is **opt-in by declaring units**: a model that declares units on N
175175
## Special features
176176

177177
- **`src/analysis.rs`** - High-level model analysis API: `analyze_model(project, db, source_project, model_name)` bundles compilation, LTM loop discovery, and dominant-period calculation into a single `ModelAnalysis` result. The caller provides a `SimlinDb` and `SourceProject` (already synced); all compilation and structural analysis use the incremental salsa path. Uses an element-level causal graph (`model_element_causal_edges` + `causal_graph_from_element_edges_with_modules`) so arrayed models get per-element loop detection AND the graph carries the module sub-graphs + variable map the discovery-mode per-exit-port pathway recompute needs (GH #698 -- the bare `causal_graph_from_element_edges` leaves both empty, which silently disabled the recompute on this production path). Passes `LtmSyntheticVar` metadata and datamodel dimensions to `discover_loops_with_graph` for A2A link score expansion, a `LinkExpansionContext` (built by the public `build_link_expansion_context`: per-variable declared dims + the dimension-mapping context) so the discovery from-node projection matches the element graph (GH #754), plus an emission-derived `SubModelOutputPorts` map built by the public `build_sub_model_output_ports` (one `db::ltm::sub_model_output_ports` call per project model) so the discovery per-exit-port recompute enumerates pathway indices against the IDENTICAL project-wide port set the sub-model emitted against -- not a parent-scoped re-derivation that shifts when another project model reads an extra output port (GH #698 / PR #705 r3353097150). A model that cannot be *compiled* for LTM analysis (a malformed equation, an unresolved reference, or the GH #486 non-Euler hard-fail) still returns `Ok` with the model snapshot intact and empty loop fields, but the actionable compile-error message is carried out in `ModelAnalysis::analysis_error` rather than swallowed by an `.ok()?` -- so "could not analyse" is distinguishable from "no loops" (GH #660), and the MCP `read_model`/`edit_model` surfaces re-expose it as `analysisError`. A non-compile structural edge case (model absent from the causal graph, post-simulation discovery bail) degrades gracefully to empty loops with `analysis_error == None`.
178-
- **`src/ltm/`** - Loops That Matter: core data types and causal graph construction, organised as a directory module. `mod.rs` re-exports the public API at `crate::ltm::*` (so external callers compile unchanged) and defines `MAX_LTM_SCC_NODES` plus the top-level `detect_loops` helper. Submodules:
178+
- **`src/ltm/`** - Loops That Matter: core data types and causal graph construction, organised as a directory module. `mod.rs` re-exports the public API at `crate::ltm::*` (so external callers compile unchanged) and defines `MAX_LTM_SCC_NODES`. Submodules:
179179
- `types.rs` - public LTM vocabulary: `LinkPolarity`, `Link`, `Loop`, `LoopPolarity`, `TruncatedByBudget`, plus the `normalize_module_ref` / `is_synthetic_node_name` helpers (`is_synthetic_node_name` = "name carries the reserved `$⁚` synthetic prefix `SYNTHETIC_NODE_PREFIX`"; the broad generalization of `ltm_agg::is_synthetic_agg_name`, used by `collapse_synthetic_links`). `LoopPolarity` is determined by counting negative links in a cycle. The per-reference shape distinction is encoded in `Link.from` / `Link.to` strings, not as a separate field: a cross-dimensional FixedIndex / full-reduce edge carries an element subscript on `from` (`"pop[nyc]"`); a cross-element edge that visits one slot of an A2A target carries it on `to` (`"mp[boston]"`); a loop through an inlined reducer traverses `from[d] → $⁚ltm⁚agg⁚{n} → to[e]` (the agg name is subscript-free, and is trimmed from the reported loop).
180180
- `partitions.rs` - `CyclePartitions` groups loops into stock-to-stock SCCs over the parent-level stock graph; bundles the generic Tarjan SCC used by `compute_cycle_partitions`.
181181
- `polarity.rs` - static polarity analysis on `Expr2` ASTs: `analyze_link_polarity` plus the small expression predicates it leans on (`flip_polarity`, `expr_references_var`, `literal_sign`/`provable_value_sign` -- both seeing through unary negation, since the lexer takes no leading sign and a parsed `-5` is `Op1(Negative, Const(5))` -- `analyze_graphical_function_polarity`, etc.). The Mul one-side arm applies the SD **positive-value labeling convention** to a bare named co-factor (`cofactor_value_sign`): `net_growth = population * fractional_growth` labels `population -> net_growth` Positive, the reading every CLD gives it, and the same convention the Div arm has always applied to `share = pop over total`. A provable co-factor sign (a literal, or a variable whose whole equation is one) beats the convention; a COMPOUND co-factor (`1 - pop over K`) stays Unknown -- its value sign is derived, not conventional, and the single-equation logistic class genuinely flips mid-run. The convention is a labeling decision, not a proof: runtime loop-score reclassification remains the ground truth and overrides it (pinned by the Rux fixtures, which construct a bare co-factor whose value really does flip sign). Per-element graphical functions (#502): when an arrayed source feeds an arrayed graphical-function target, the per-element `tables` list on `Variable::Var` is folded into one link polarity (`fold_per_element_table_polarity` over each element's `Table`); the multi-dim case stays conservatively `Unknown`. The strict-monotonicity check classifies each segment by its **slope** (dy over dx) against a tolerance of `1e-6 * (y_max - y_min) / avg_dx` (floored at `1e-12`; GH #536), where `avg_dx` is the average x-spacing (`x_span` divided by the number of segments). The per-segment noise threshold is `tolerance * dx = 1e-6 * (y_max - y_min) * dx / avg_dx`: on uniformly-spaced tables every `dx == avg_dx` so the threshold reduces EXACTLY to `1e-6 * (y_max - y_min)` -- the same y-range-relative dy epsilon #492 used, preserving import-noise tolerance for finely-sampled tables; for non-uniform tables the threshold scales proportionally with segment width so a narrow steep segment is still caught. A degenerate vertical segment (`x[i] == x[i-1]`) is skipped if it is a redundant duplicate point and bails to `Unknown` if it is a genuine two-outputs-for-one-input step. Numeric-import noise on a near-flat lookup arm still does not flip a monotone table to `Unknown`.

src/simlin-engine/src/db/analysis.rs

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2837,42 +2837,34 @@ fn detected_loop_from_loop(l: &crate::ltm::Loop, pin_name: &str) -> DetectedLoop
28372837
/// (model-only) surface. The structural FFI takes no `Results` and reports R/B
28382838
/// at confidence 1.0 / U at 0.0; the runtime FFI builds the same exhaustive
28392839
/// loop set and calls this helper over the completed sim's `loop_score`
2840-
/// series, so the exhaustive surface can finally report `Mostly*` (Rux/Bux) or
2841-
/// a runtime sign flip. (Now that GH #495 surfaces all five polarity variants
2842-
/// along with the confidence verbatim across the FFI, there is no longer a
2843-
/// coalescing/confidence-drop at the boundary.) The pysimlin `Run.loops`
2844-
/// surface still reclassifies via its own Python `LoopPolarity.from_runtime_scores`
2845-
/// mirror (slot-0 only -- see the A2A note below); pysimlin exposes the
2846-
/// all-slots engine path separately as `Run.loops_runtime`. The engine
2840+
/// series, so the exhaustive surface can report `Mostly*` (Rux/Bux) or a
2841+
/// runtime sign flip. (GH #495 surfaces all five polarity variants along with
2842+
/// the confidence verbatim across the FFI, so there is no
2843+
/// coalescing/confidence-drop at the boundary.) pysimlin's `Run.loops` rides
2844+
/// this same helper -- bound as `Sim.get_loops_runtime` -- so Python performs
2845+
/// no reclassification of its own and the classification rules live only in
2846+
/// [`crate::ltm::LoopPolarity::from_runtime_scores`]. The engine
28472847
/// `analyze_model` / MCP surface is discovery-based and reclassifies through
28482848
/// the `FoundLoop` path.
28492849
///
2850-
/// # A2A semantics differ across the three reclassification sites
2850+
/// # A2A semantics differ between the two reclassification sites
28512851
///
28522852
/// `loop_partitions` is the per-loop slot->partition map carried on
28532853
/// `LtmVariablesResult::loop_partitions`; its slot-vector length is the
28542854
/// `loop_score` series' slot count. For an A2A (per-element) loop this helper
28552855
/// **concatenates every element slot's series into one sample set** and
28562856
/// classifies the mixed result: if any element of the loop is balancing while
28572857
/// another is reinforcing the loop classifies `Undetermined` (a deliberate
2858-
/// "the loop's sign is not uniform across the array" reading). This is NOT the
2859-
/// same input construction the other two sites use, so do not claim they
2860-
/// agree:
2861-
/// - **pysimlin `Run.loops`** reads `get_series("$⁚ltm⁚loop_score⁚{id}")`,
2862-
/// which resolves to **slot 0 only** (the dominant/first element), so an
2863-
/// A2A loop is classified from a single element's series.
2864-
/// - **discovery** (`ltm_finding`) classifies each `FoundLoop` from its own
2865-
/// single strongest-path scalar score series.
2858+
/// "the loop's sign is not uniform across the array" reading). This is NOT
2859+
/// the input construction discovery uses, so do not claim they agree:
2860+
/// **discovery** (`ltm_finding`) classifies each `FoundLoop` from its own
2861+
/// single strongest-path scalar score series.
28662862
///
2867-
/// All three share the *scalar* semantics (`from_runtime_scores`'s NaN/zero
2863+
/// Both sites share the *scalar* semantics (`from_runtime_scores`'s NaN/zero
28682864
/// filter; all-positive -> Reinforcing, all-negative -> Balancing, mixed
2869-
/// dominant >= threshold -> Mostly*, otherwise Undetermined) and agree exactly
2870-
/// on a scalar loop; they diverge only in how an A2A loop's multiple element
2871-
/// slots are reduced to one classification. The sim-bearing FFI consumer
2872-
/// (`simlin_analyze_get_loops_runtime`, GH #679) deliberately exposes THIS
2873-
/// all-slots reading -- pysimlin surfaces it as `Run.loops_runtime`, keeping
2874-
/// the slot-0 `Run.loops` path unchanged -- so the two A2A readings now coexist
2875-
/// rather than one being reconciled into the other.
2865+
/// dominant >= threshold -> Mostly*, otherwise Undetermined) and agree
2866+
/// exactly on a scalar loop; they diverge only in how an A2A loop's multiple
2867+
/// element slots are reduced to one classification.
28762868
pub fn reclassify_loops_from_results(
28772869
loops: &mut [DetectedLoop],
28782870
results: &crate::Results,

src/simlin-mcp/src/main.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -135,12 +135,11 @@ mod tests {
135135
// setuptools-scm derives it from the `pysimlin-v*` tag itself (see
136136
// `tag_regex` in src/pysimlin/pyproject.toml) -- so the tag is the only
137137
// thing this can be checked against, and the check needs the tags to be
138-
// present locally. CI checks out with `actions/checkout@v4` and no
139-
// `fetch-depth`, i.e. a shallow clone with no tags, so this guard runs
140-
// only on a developer's full clone and is a no-op in CI. That is a
141-
// deliberate limitation, not an oversight: making it fail on an empty tag
142-
// list would break every CI run. The skip is announced rather than silent
143-
// so a run that unexpectedly finds no tags is attributable.
138+
// present locally. CI's Build-job checkout sets `fetch-tags: true`
139+
// specifically so this guard runs there. A clone without the tags (e.g. a
140+
// developer's fresh shallow clone) skips rather than fails -- announced
141+
// rather than silent, so a run that unexpectedly finds no tags is
142+
// attributable.
144143
#[test]
145144
fn pysimlin_version_matches_latest_tag() {
146145
let output = std::process::Command::new("git")
@@ -151,8 +150,9 @@ mod tests {
151150
if !output.status.success() || tags.trim().is_empty() {
152151
eprintln!(
153152
"SKIPPING pysimlin_version_matches_latest_tag: no pysimlin-v* tags are \
154-
visible (a shallow clone, as CI produces, fetches no tags). Run \
155-
`git fetch --tags --unshallow` to exercise this guard."
153+
visible. CI fetches tags (fetch-tags: true in ci.yaml), so this skip is \
154+
expected only on a local clone without them; run `git fetch --tags` to \
155+
exercise this guard."
156156
);
157157
return;
158158
}

0 commit comments

Comments
 (0)