fix(core): break the synaptic-plasticity import cycle (#233)

[cdeust]

Closes #233.

The defect

mcp_server/core/synaptic_plasticity.py held the Tsodyks-Markram implementation and back-imported its two siblings at the bottom of the file behind # noqa: E402, while synaptic_plasticity_hebbian.py and synaptic_plasticity_stochastic.py imported names back out of it. Whichever of the three was imported first in a fresh interpreter decided whether the import worked.

Reproduced on main at f606c37:

$ for m in mcp_server.core.synaptic_plasticity mcp_server.core.synaptic_plasticity_hebbian mcp_server.core.synaptic_plasticity_stochastic; do
    printf '%-55s ' $m; .venv/bin/python -c "import $m" 2>&1 | tail -1; done
mcp_server.core.synaptic_plasticity
mcp_server.core.synaptic_plasticity_hebbian    ImportError: cannot import name 'apply_hebbian_update' from partially initialized module 'mcp_server.core.synaptic_plasticity_hebbian' (most likely due to a circular import)
mcp_server.core.synaptic_plasticity_stochastic ImportError: cannot import name 'apply_stochastic_hebbian_update' from partially initialized module 'mcp_server.core.synaptic_plasticity_stochastic' (most likely due to a circular import)

Hoisting only the two weight constants — the literal wording of the issue's criterion 2 — would not have fixed _stochastic, which also imports SynapticState and four STP functions from the facade. Criterion 1 names both modules, so the fix is the leaf-module extraction below, not a constants-only hoist.

The change

Fowler Move Function, applied to satisfy the dependency rule (coding-standards §2.2: a leaf must not import its facade).

• NEW mcp_server/core/synaptic_plasticity_stp.py — the Tsodyks-Markram constants, SynapticState, compute_effective_release_probability, stochastic_transmit, update_short_term_dynamics, _recover_between_spikes, _apply_spike, compute_noisy_weight_update, phase_modulate_plasticity. Imports math, random, dataclasses — nothing else. A true leaf.
• synaptic_plasticity_hebbian.py / synaptic_plasticity_stochastic.py retarget their imports at the leaf. Their diffs are import statements only — zero executable lines changed.
• synaptic_plasticity.py becomes a pure re-export facade (59 lines) with the unchanged 14-name __all__. Both # noqa: E402 markers are deleted: with no cycle they are no longer true statements about the file (§9).
• Consumers are untouched. All five (handlers/consolidation/plasticity.py:11, tests_py/benchmarks/benchmark_harness.py:553, tests_py/core/test_stochastic_transmission.py, tests_py/core/test_synaptic_plasticity.py, tests_py/core/test_ablation_hooks.py:195) import via the facade or _hebbian, both of which keep working. No consumer imports _MIN_WEIGHT/_MAX_WEIGHT from the facade (verified by grep across mcp_server/ tests_py/ benchmarks/ scripts/).

The move is byte-identical

$ git show HEAD~1:mcp_server/core/synaptic_plasticity.py | sed -n '33,227p' > orig_body.txt
$ tail -n +36 mcp_server/core/synaptic_plasticity_stp.py > new_body.txt
$ diff orig_body.txt new_body.txt && echo "IDENTICAL"
IDENTICAL: moved code is byte-for-byte HEAD lines 33-227

No constant, equation or # source: comment was touched.

Completion Ledger (§13.2)

Path enumeration — every path the diff introduces

Path introduced	Evidence
synaptic_plasticity_stp importable as a leaf (no sibling imports)	test_import_isolation.py::test_module_imports_first_in_a_fresh_interpreter[mcp_server.core.synaptic_plasticity_stp]
synaptic_plasticity facade re-exports all 14 __all__ names	test_stochastic_transmission.py + test_synaptic_plasticity.py import 12 of the 14 through the facade and pass (43 + 23 tests); handlers/consolidation/plasticity.py imports the 13th (apply_hebbian_update), covered by tests_py/handlers/consolidation
_hebbian importable first	...test_module_imports_first_in_a_fresh_interpreter[mcp_server.core.synaptic_plasticity_hebbian]
_stochastic importable first	...test_module_imports_first_in_a_fresh_interpreter[mcp_server.core.synaptic_plasticity_stochastic]
facade importable first (regression guard on the old happy path)	...test_module_imports_first_in_a_fresh_interpreter[mcp_server.core.synaptic_plasticity]
subprocess returncode != 0 arm	Asserted; observed firing on the pre-fix tree (3 failed / 1 passed, quoted below)
subprocess stderr non-empty arm	Asserted (assert result.stderr == ""); the pre-fix run's failure output is the stderr the assert message prints
_FixedRandom.random() stub path	TestStochasticTransmit::test_draw_exactly_equal_to_p_does_not_transmit
_FixedRandom.gauss() stub path + last_sigma recording	TestNoisyWeightUpdate::test_no_evidence_uses_the_full_noise_scale_and_adds_it, ::test_sigma_scales_by_inverse_sqrt_of_evidence
compute_noisy_weight_update access_count <= 0 branch	::test_no_evidence_uses_the_full_noise_scale_and_adds_it (asserts sigma == 1.0)
compute_noisy_weight_update else branch (1/sqrt(n))	::test_sigma_scales_by_inverse_sqrt_of_evidence (asserts sigma == 0.5 at n=4)
update_short_term_dynamics no-spike branch + is_access default	TestShortTermDynamics::test_between_spike_recovery_exact_values (calls without is_access; asserts u == 0.274406, x == 0.311434, access_count carried, hours_since_last_access == 0.3)
update_short_term_dynamics spike branch	TestShortTermDynamics::test_spike_exact_values (asserts u == 0.6, x == 0.2, access_count == 3)
phase_modulate_plasticity LTP branch + is_ltp default	TestPhaseModulation::test_ltp_envelope_exact_off_peak
phase_modulate_plasticity LTD branch	TestPhaseModulation::test_ltd_envelope_exact_off_peak
compute_effective_release_probability unclamped path	TestEffectiveReleaseProbability::test_u_eff_equals_U_plus_u_times_one_minus_U

§13.1 checklist

A. Correctness & behavior

• A1 Happy paths tested end-to-end, output quoted — full suite 6388 passed, 30 subtests passed in 562.85s, exit 0.
• A2 Edge cases: the import-order edge case IS the defect, and all four orderings are exercised. Numeric boundaries mapped to tests: r == p exactly (test_draw_exactly_equal_to_p_does_not_transmit), access_count == 0 vs > 0, hours_elapsed == 0 vs > 0, theta on-peak (pre-existing) vs off-peak (new). The clamp bounds [0.05, 0.95] keep their pre-existing tests.
• A3 Failure paths: the regression test's two failure arms (non-zero exit, non-empty stderr) both fire on the pre-fix tree — quoted below. No new error arms are introduced in production code; the relocated code has none.
• A4 N/A — no trust boundary in this diff; the modules are pure functions on floats/dataclasses called from within core.
• A5 Invariants unchanged (the moved code is byte-identical); update_short_term_dynamics still returns a new SynapticState and does not mutate its argument. Partial failure: N/A, no I/O, no state.
• A6 N/A — every function is pure and single-shot by construction.

B. Concurrency — B1/B2/B3 N/A: no concurrency touched; the new module has no shared mutable state (module-level names are float constants, never rebound).

C. Resources & performance

• C1 N/A — no new loop, collection or query; the diff moves existing straight-line arithmetic.
• C2 N/A — no file descriptors, connections or processes in production code. The regression test spawns 4 short-lived subprocesses with a 120 s timeout each; measured 4 passed in 0.61s.
• C3 N/A — no hot path touched; identical bytecode in the moved functions.

D. Security — D1/D2/D3 N/A: no SQL/shell/path construction, no untrusted input, no secrets.

E. Interfaces & compatibility

• E1 Additive: __all__ is unchanged (14 names, same order). One new module is added; nothing is removed from the public surface.
• E2 Downstream consumers identified by name (list above) and each verified: tests_py/handlers/consolidation, test_ablation_hooks.py, test_synaptic_plasticity.py, test_stochastic_transmission.py — 160 passed in 27.02s. benchmark_harness.py imports from the facade, which keeps the same names.
• E3 N/A — no persisted data or format change.
• E4 N/A — pure arithmetic, no paths/encoding/process spawning in production code. The test's subprocess.run([sys.executable, "-c", ...], cwd=repo_root) uses no shell and no platform-specific path syntax; sys.executable and pathlib are the portable forms, and CI runs the suite on Windows.

F. Observability & operations

• F1 The failure signal of this change IS the import error, and the regression test asserts it by exit code AND by stderr content; the nominal path asserts stderr is empty, so a module that starts printing warnings at import time fails the gate.
• F2 N/A — no degraded mode added or removed.

G. Tests

• G1 Ledger above is complete.
• G2 Regression test fails on the pre-fix code — quoted below.
• G3 Deterministic and isolated: no sleeps, no shared paths, no order dependence. The subprocess tests are independent by construction (a new interpreter each). The _FixedRandom stub removes the sampling nondeterminism the pre-existing tests relied on. Full suite passed twice, at default parallelism.
• G4 Negative assertions present: assert result.stderr == "" (absence of output IS the contract), and assert stochastic_transmit(...) is False at the r == p boundary.
• G5 Full suite run locally, output quoted; ruff format --check and ruff check pass on the whole tree.

H. Code quality & delivery

• H1 §2.2 dependency rule is what this PR restores. §4 sizes: _stp.py 229 lines, facade 59, longest new function 30 lines, max 4 params, nesting depth 2. §8: no constant added or altered.
• H2 No dead code, no debug leftovers; the two # noqa: E402 markers are removed because the condition that justified them is gone.
• H3 Naming follows the existing synaptic_plasticity_* convention. One language per file.
• H4 CHANGELOG.md entry added; docs/module-inventory.md gains the new module and corrects the facade's description.
• H5 Single logic commit; the doc/badge count updates ride with it because the gate requires them to agree with the same tree.
• H6 CI on the exact pushed tree — see the check results on this PR.
• H7 Boy-scout (§14) — see below.

Verification (real output)

1. The defect, before and after. Pre-fix (main, f606c37) output is quoted at the top. On this branch:

mcp_server.core.synaptic_plasticity                     OK
mcp_server.core.synaptic_plasticity_stp                 OK
mcp_server.core.synaptic_plasticity_hebbian             OK
mcp_server.core.synaptic_plasticity_stochastic          OK

2. The regression test fails on the pre-fix source (G2). Sources reverted to HEAD, _stp.py parked, new test file kept:

$ uv run pytest tests_py/core/test_import_isolation.py -q
...
E         ImportError: cannot import name 'apply_stochastic_hebbian_update' from partially
E         initialized module 'mcp_server.core.synaptic_plasticity_stochastic' (most likely
E         due to a circular import)
E       assert 1 == 0
FAILED ...[mcp_server.core.synaptic_plasticity_stp]
FAILED ...[mcp_server.core.synaptic_plasticity_hebbian]
FAILED ...[mcp_server.core.synaptic_plasticity_stochastic]
3 failed, 1 passed in 1.44s

And on this branch: 4 passed in 0.61s.

3. Behaviour unchanged (issue criterion 3).

$ uv run pytest tests_py/core/test_synaptic_plasticity.py tests_py/core/test_stochastic_transmission.py \
                tests_py/core/test_ablation_hooks.py tests_py/handlers/consolidation -q
160 passed in 27.02s

(run before the 8 new tests were added, i.e. the pre-existing assertions alone, unmodified.)

4. Full suite + gates.

$ uv run pytest --tb=short -q
6388 passed, 30 subtests passed in 562.85s (0:09:22)

$ uv run ruff format --check .
1060 files already formatted
$ uv run ruff check
All checks passed!          # ruff 0.15.20, the ci.yml:386 pin

5. Mutation testing (§12.1), and a correction to the planned invocation.

The planned command pairs _stp.py with test_synaptic_plasticity.py. That pairing is vacuous — test_synaptic_plasticity.py covers the Hebbian/STDP surface, not STP — and mutmut says so itself:

$ scripts/mutation_check.sh tests_py/core/test_synaptic_plasticity.py mcp_server/core/synaptic_plasticity_stp.py
done in 2539ms (1 files mutated, 519 ignored, 0 unmodified)
Stopping early, because we could not find any test case for any mutant.
It seems that the selected tests do not cover any code that we mutated.

Run against the file that actually exercises STP, the first result was 29 survivors of 128:

$ scripts/mutation_check.sh tests_py/core/test_stochastic_transmission.py mcp_server/core/synaptic_plasticity_stp.py
128/128  🎉 99  🙁 29

The surviving mutants showed the published equations were pinned only by inequality assertions ("u decays", "x recovers"), e.g. u * (1.0 - _U_INCREMENT) → u / (1.0 - _U_INCREMENT), math.exp(-h / _TAU_F_HOURS) → math.exp(-h * _TAU_F_HOURS), round(u, 6) → round(u, 7), delta_w + noise → delta_w - noise, (rng or random) → (rng and random) (which silently ignored the caller's rng). 8 exact-value tests kill 26 of the 29. After:

>>> survivors (must be empty, or documented equivalents):
    ..._stp.x__recover_between_spikes__mutmut_1: survived
    ..._stp.x_compute_noisy_weight_update__mutmut_2: survived
    ..._stp.x_phase_modulate_plasticity__mutmut_8: survived

All three are equivalent mutants, each with its rationale recorded in the test module's docstring:

1. _recover_between_spikes: hours_elapsed <= 0 → < 0. Differs only at hours_elapsed == 0, where the recovery formulas reduce to the identity (u*exp(0) == u, 1-(1-x)*exp(0) == x). The residual is IEEE-754 reassociation round-off (~4e-17) and the caller rounds to 6 decimals, so no public call can observe it.
2. compute_noisy_weight_update: access_count <= 0 → <= 1. Differs only at access_count == 1, where both branches yield evidence_factor == 1.0 exactly.
3. phase_modulate_plasticity: cos(2π(θ − 0.25)) → − 1.25. Exactly one period of cosine; measured max difference 2.5e-16 across θ ∈ {0, 0.125, 0.25, 0.4, 0.5, 0.75, 0.9, 1.0}. Pinning it would assert float noise, not behaviour.

The other three touched files are not mutated, because their diffs contain no executable line: _hebbian and _stochastic change import statements only, and the facade is imports + the unchanged __all__. git diff for both siblings is in the commit and shows exactly that.

6. Test-count / badge gate.

$ N=$(uv run pytest --collect-only -q | sed -n 's/^\([0-9][0-9]*\) tests collected.*/\1/p' | tail -1)
6388
$ uv run python scripts/generate_repo_badges.py --test-count 6388
wrote assets/badge-tests.svg
1 of 5 badge(s) changed
$ uv run python scripts/check_doc_claims.py --test-count 6388
doc claims OK (1 declared not-a-claim exemption(s))
  CONTRIBUTING.md:36: exempt from the tests claim
$ uv run python scripts/generate_repo_badges.py --check --test-count 6388
badges OK (5 checked)

6376 → 6388 (+4 import-isolation cases, +8 mutation-killing cases). The ten prose sites in README.md, CONTRIBUTING.md, CLAUDE.md, docs/ASSURANCE-CASE.md and .bestpractices.json were updated to agree; .bestpractices.json still parses as JSON.

§14 Boy-scout

• synaptic_plasticity_hebbian.py:45-49 carries _STDP_COINCIDENCE_HOURS = 0.001 with # source: pre-existing tuned value ... provenance not recorded at introduction. That is an accepted §8 form already in the file. Left as-is; inventing a citation would be worse than the honest note.
• Deferred with an issue number: typecheck gate is resolver-dependent: pyright reports 1 error under uv.lock, 0 under CI's pip resolution (ast_parser.py:88) #249. python -m pyright mcp_server/ reports 1 error in this worktree, at mcp_server/core/ast_parser.py:88 (str not assignable to SupportedLanguage) — a file this diff does not touch. A paired control proves it is not mine: reverting the four source files and re-running pyright in the same environment reports the identical single error. It does not appear in CI (ci.yml typecheck is green on f606c37) because CI's pip install -e ".[...]" resolves tree-sitter-language-pack to the newest release under <1.14 (1.13.5 today) while uv.lock pins 1.6.2, and the two stubs disagree. That makes the zero-diagnostic gate resolver-dependent, which is a real defect — filed as typecheck gate is resolver-dependent: pyright reports 1 error under uv.lock, 0 under CI's pip resolution (ast_parser.py:88) #249 with reproduction and acceptance criteria, outside this change's blast radius (different subsystem, and the gate this diff triggers reports it clean).

Not deviated from the task definition (§15)

Every clause of #233 is met in this PR: criterion 1 (all three modules import first — plus the new leaf, four in total, each pinned by a subprocess test), criterion 2 (the shared symbols now live in a module that imports no sibling — implemented as the full leaf extraction, because hoisting only the two weight constants would have left _stochastic broken, as shown above), criterion 3 (behaviour unchanged, 160 pre-existing tests pass untouched, and the moved code is byte-identical). Nothing is deferred to a follow-up.

Reuse note for #237

tests_py/core/test_import_isolation.py is written as the single home for this check: fix-consolidation-import-cycle-237 must append its four module names to CYCLE_PRONE_MODULES in this file rather than create a second one.

🤖 Generated with Claude Code

https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u