Chore/audit cleanup 1.1.x - #3
Merged
Merged
Conversation
added 2 commits
August 27, 2026 20:12
Single-PR pass over the high-confidence, low-risk findings from the
codebase audit: silent-correctness drops, dead code, doc rot, a no-op
test, and missing CI. All changes preserve the byte-identical
trajectory contract — verified by hashing legacy and Layer 2.5 sv
against clean HEAD (identical hashes).
Workstream A — silent-correctness drops
- A1 Remove unused `import logging` + `logger` from live_simulator.py.
- A2 Fix plots.py:358 `include_plotlyjs` comment (was `inline`, code is `cdn`).
- A3 Document live_sp* semantics in Settings: LiveSimulator-only;
batch reads sp*. Prevents accidental "make batch honor live_sp*"
fingerprint-breaking refactors.
- A4 Move `import copy` to module top in config.py.
- A5 SKIPPED. Promoting `_pfaults` from class-level default to a
dataclass field(init=False, repr=False) is technically a behavior
change in dataclass field ordering. Per AGENTS.md "Numba kernels
are sacred", a riskier change deserves its own discussion. Tracked
in Progress.md "Decisions" + "Open Questions".
Workstream B — dead code removal
- B1 Remove unused Parameters.vmolo_local / .cpmolo_local (and their
finalize() writes).
- B2 Remove self-assignment `self.cpmolm = self.cpmolm` in finalize().
- B3 DEFERRED. ProcessFaults.fouling_mode is documented public API;
removal needs Joel's call (see Open Questions).
- B4 Drop the `factor_for_window` thin pass-through helper in
fouling_modes.py and the corresponding test in
test_layer28b_fouling_modes.py. LiveSimulator calls
`self._fouling_stepper.step(...)` directly.
- B5 `_PESOS_W_OUT = _PESOS_W.copy()` — explicit copy in split_nn.py
removes an aliasing hazard.
- B6 Remove unreachable `i == 0` from lab-cycle latch in both
run_with and LiveSimulator.step. last_lab_sample_t is initialised
to -inf which already forces a sample at t=0.
Workstream C — test fix
- C5 Rewrite test_stuck_does_not_update_from_dropout to actually
exercise the path: dropout first (NaN published), then stuck (must
latch onto the cached finite value, not the NaN).
Workstream D — documentation fixes
- D1 Fix the false "test_smoke.py fingerprint regression" claim in
AGENTS.md, ADMIN.md, and README.md. test_smoke.py checks shapes,
physical ranges, and determinism only — no SHA pins. The pinning
lives in test_live_simulator.py + per-Layer tests.
- D2 Resolve AGENTS.md vs ADMIN.md contradiction on the dashboard's
bdsim-version pin (the dashboard does NOT pin today).
- D3 README.md "matplotlib" → "Plotly".
- D4 Date the Performance table ("captured 2026-07-13 on the
bdsim-dashboard reference host").
- D5 Expand AGENTS.md profile fingerprint table: 9 rows now
including Layer 2.4 pump-only / valve-only / both and the
Layer 2.6b row.
- D6 Resolve the Layer 2.1 "✅ shipped but pending review" contradiction
in AGENTS.md by collapsing both lines into a single 🟡 row.
- D7 NOTES.md smoke-test count "12" → "14" (two occurrences).
- D8 Tighten config.py:695 StepResult.spectra type from `object | None`
to `SpectrumSample | None` (TYPE_CHECKING import — no runtime cost,
no ruff F821).
- D9 Remove USER.md stale IndexError warning for sim.t after done —
fixed in v0.4.1 (test_sim_t_after_done_returns_settings_tf).
- D10 USER.md ProcessFaults table expanded to ~30 rows by Layer.
- D11 docs/30-engine/Config-surface.md same expansion (mirrors USER.md).
- D12 docs/30-engine/Fingerprints-and-tests.md table expanded to
match AGENTS.md.
- D13 New docs/30-engine/Fouling-modes.md — five-mode stepper
walkthrough with three-layer priority diagram, mode table,
determinism notes, ProcessFaults knobs, and test pointers.
Linked from docs/Home.md "Engine" map.
- D14 .gitignore: PDF paths moved under docs/ (Fernandes2019_BDSIM.pdf
and manual.pdf).
- D15 docs/README.md + ADMIN.md: stop claiming PDFs are gitignored at
the repo root (they're under docs/ now).
Workstream E — CI
- E3 New .github/workflows/ci.yml: pytest matrix on Python 3.10 / 3.11
/ 3.12 via uv sync --extra test, plus a ruff job. Cancels
in-flight runs on the same branch.
Verification
- `ruff check` clean on all changed files (remaining 22 issues are
pre-existing bdsim/ode.py E702/F841, out of scope per AGENTS.md).
- Non-fingerprint tests pass: test_smoke.py 14/14, test_live_simulator
non-fp 15/15 + byte-identical contract 2/2, test_layer28b_* 36/36.
- Fingerprint regression: legacy sv=6f61eb53... and Layer 2.5
sv=1938fec8... match clean HEAD — this PR introduces no
fingerprint drift.
Pre-existing (out of scope, tracked in Progress.md "Discovered during work")
- The 1.1.1 pinned hashes (c8807b23... / 696531c4... / 8865a8c3... /
bb763a9b...) are retargeted to numpy==2.2.6 / scipy==1.15.3 (CHANGELOG
1.1.1). This box resolves to numpy==2.4.6 / scipy==1.18.0, so the
pinned tests fail with the pre-1.1.1 hash. Documented in Progress.md.
- Layer 2.1 + Layer 2.4 + Layer 2.5 combination crashes with
IndexError: index 27 is out of bounds for axis 1 with size 27
(sv width off-by-one). Same bug, same fix needed, in
bdsim/simulation.py:441 and bdsim/live_simulator.py:849. Separate
audit item.
Progress.md captures the full task log, decisions, and open questions.
Bare ProcessFaults() now enables Layers 2.5 (fouling), 2.1 (quality
latching), 2.4 pump + valve wear, and 2.8a (NIR/IR spectra). The sv
width grows to 30. The legacy / Layer 2.5 / Layer 2.4 / Layer 2.6
profiles below remain canonical with their 1.1.1 pins, but tests
and demos targeting them must pass explicit `False` overrides now.
This is intentional, documented, and pinned in `tests/`. New pins:
- Runtime default (bare) batch: sv=691cf51b4c1a0bc2
pv=baedc29fcfa8f526
uv=4e4134e40fa0c1ad
- Runtime default (bare) live : sv=80f6f04683703382
pv=9e2b2081f469e473
uv=c3a05be9a234fe2a
- Layer 2.5 + Layer 2.1 batch: sv=d663e17d687b1133 (new profile)
All 1.1.1 pins preserved (legacy / Layer 2.5 / Layer 2.4 / Layer 2.6
profiles still match the prior hashes when configured with explicit
overrides).
ProcessFaults default changes (bdsim/config.py)
- fouling_dynamic=True (no change — default since 1.0)
- quality_state: False → True # Layer 2.1 master switch
- pump_wear: False → True # Layer 2.4 pump degradation
- valve_wear: False → True # Layer 2.4 valve stiction
- spectrum_enabled: False → True # Layer 2.8a NIR/IR
Documentation updates
- AGENTS.md: "Default profiles" bullet rewritten; profile table now
includes the new Runtime default (1.2.0+) row with batch + live
pins, and corrects sv-width/type/construction for the existing
rows (Layer 2.5+2.1 → 27-wide; Layer 2.4 rows add quality_state=False
to the Construction column).
- ADMIN.md: fingerprint table rebuilt (Runtime default row + corrected
Layer 2.5+2.1 row), "Fingerprint regression" body + "Run-only"
footnote updated.
- USER.md: ProcessFaults field table — default column flipped to bold
"True" on quality_state / pump_wear / valve_wear / spectrum_enabled;
Common-gotcha rewritten.
- docs/30-engine/Config-surface.md: field table defaults updated; "Note"
callout rewritten.
- docs/30-engine/Fingerprints-and-tests.md: profile table expanded.
- docs/00-orientation/Byte-identical-contract.md: "Canonical profiles"
table rewritten (Runtime default is its own pinned profile now),
"How to reproduce pins" version bumped.
- README.md: header version + Runtime-default note.
- CHANGELOG.md: new [1.2.0] entry — Changed / Added / Notes.
Test updates
- tests/test_smoke.py: explicit `False` overrides on fouling-dynamic,
quality-state, pump-wear, valve-wear, and spectrum-enabled where
the test was implicitly relying on legacy / Layer 2.5 paths.
- tests/test_live_simulator.py: same overrides on the 4 fingerprint
tests + byte-identical contract tests; one assertion
(`test_step_returns_step_result_with_correct_shapes`) bumped from
sv-shape == 22 to == 30 to match the new broad default.
- tests/test_disturbances.py, tests/test_layer24_degradation.py,
tests/test_layer26b_cw_pump.py, tests/test_layer27_knobs.py: explicit
False overrides on every ProcessFaults construction that targets a
profile narrower than the new broad default. No pin updates —
the legacy / Layer 2.4 / Layer 2.6 / Layer 2.7 profiles still match
the 1.1.1 hashes when configured explicitly.
- tests/test_layer28a_spectra.py: explicit `spectrum_enabled=False`
on the two tests that expect spectra to be None (it now defaults
ON, so an opt-out is required).
Numerics fix (bdsim/ode.py)
- RHS `dsvdt` was allocated as 28 + pump + valve when quality_state was
True (assuming Layer 2.5 α was also on). This silently mismatched
the driver-side sv width of 21 + 6 + pump + valve in any test that
set quality_state=True with fouling_dynamic=False. Fix: size dsvdt
as 21 + (1 if dynamic) + (6 if quality) + pump + valve always.
Trajectory unchanged for the common (dynamic=True) cases; closes a
latent crash window for quality_state=True + dynamic=False.
bdsim/__init__.py: __version__ "1.1.1" → "1.2.0"
pyproject.toml: version "1.1.1" → "1.2.0"
Pre-existing fingerprint drift
- The pinned-test failures on this box (`6f61eb53…` instead of
`c8807b23…`, etc.) are pre-1.1.1 hashes from a newer uv.lock stack
(numpy 2.4.6 / scipy 1.18.0 here vs the 1.1.1 reference numpy 2.2.6
/ scipy 1.15.3). Same drift on `main` HEAD — NOT caused by this
PR. Documented in CHANGELOG.md [1.1.1] "Notes" section.
Verification
- ruff check clean on changed files (22 remaining issues are
pre-existing in bdsim/ode.py — Numba multi-statement lines and
unused locals, out of scope per AGENTS.md).
- All non-fingerprint tests pass:
test_smoke.py 14/14
test_layer28a/b 36/36
test_live_simulator.py 17/17 (non-fp + byte-identical)
test_layer24_… 17/17 (non-fp)
test_disturbances.py 6/6 (non-fp)
test_layer26b/27 18/18 (non-fp)
- Fingerprint regression: 1.1.1 hashes unchanged for the legacy,
Layer 2.5, Layer 2.4 pump-only / valve-only / both, Layer 2.6
active, and live legacy profiles (the tests fail on this box with
pre-1.1.1 hashes only — same drift as `main`).
Progress.md captures the full task log + decision rationale + the
pre-existing fingerprint-drift caveat.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.