fix(gate): the complexity baseline was keyed on absolute paths - #104
Merged
Merged
Conversation
The baseline was keyed on ABSOLUTE paths with the leading slash removed:
home/matt/qc/ferric/crates/ferric-cc/src/ccd.rs::ccd@24
`rust-code-analysis-cli` mirrors each input's absolute path into its
output tree, so `os.path.relpath(fpath, tmp)` recovers the absolute
path, not a repo-relative one. All 6104 baseline keys carry one
checkout's directory.
Consequence: the gate is unusable in any git worktree. Every function
looks NEW, so every push is measured against the bare
NEW_FUNCTION_CC_CEILING instead of against its own recorded history --
and it fails on changes that touch no Rust at all. This was found by a
commit whose entire diff was one `.config/nextest.toml` file.
Fixed by stripping ROOT from the recovered path. Keys are now
crates/ferric-cc/src/ccd.rs::ccd@24
which is portable across worktrees and machines -- the same reasoning as
[[never-commit-machine-specific-values]].
The baseline itself still holds the old keys and must be regenerated in
the MAIN checkout (`python3 scripts/complexity_gate.py
--update-baseline`), so the diff is reviewable rather than encoding a
worktree path. Doing that separately, because it also surfaces a real
regression the path bug was masking: solve_rhf is CC=176 and
solve_uhf_fockmod CC=153, both above the 150 ceiling, both grown in #88
(TRAH/AURORA). That is a genuine signal and deserves its own change, not
a silent re-baseline bundled in here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mechanically required by the previous commit: all 6104 keys were
absolute paths, so the file had to be regenerated once they became
repo-relative. 7211 functions, 0 keys starting with "home/".
What this BAKES IN, stated so it is visible in review rather than buried
in a 7000-entry diff:
crates/ferric-scf/src/rhf.rs::solve_rhf@753 CC=176
crates/ferric-scf/src/uhf.rs::solve_uhf_fockmod@360 CC=153
Both exceed NEW_FUNCTION_CC_CEILING (150). Both grew in #88
(TRAH/AURORA-SCF), and the path bug is precisely why that landed
unnoticed -- with no key matching, the gate never compared them against
their own history. The script's own header still reads "solve_rhf,
CC=134", which was true when written.
They are NOT fixed here. Recording them is the honest move: the gate is
live again, so any FURTHER growth in either function fails a push, and
simplifying two SCF drivers is not a gate fix.
MUTATION TESTED: lowering solve_rhf's baseline entry to CC=10 makes the
gate report "CC 10 -> 176 (+166)" and FAIL, so it is live rather than
passing vacuously.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthrough
ChangesComplexity path normalization
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
This was referenced Sep 19, 2026
mgoldey
added a commit
that referenced
this pull request
Sep 19, 2026
Rebased onto main as a clean set: the probe scripts, the RESULTS.md corrections, and tools/docking/united_atom.py. The old branch also carried a stale scripts/complexity_baseline.json that would have reverted #104, so this takes only the files that belong here. Contents: * run_docked_pose_scatter.py (M12), run_scorer_pose_sensitivity.py (M14), run_xtb_vs_dft_tracking.py (M16). * tools/docking/united_atom.restore_hydrogens, shared rather than copied. PDBQT is UNITED-ATOM: a docked danuglipron pose has 42 atoms and 263 electrons where the real molecule has 71 and 292. `embed_ligand_from_coords` REFUSES that (263 electrons at multiplicity 1 implies a non-integer n_alpha); `pose_fit` ACCEPTS IT SILENTLY and returns a normal-looking number for a molecule missing 29 hydrogens. M14 fixed it inside its own probe, M12's scatter did not -- which is how a per-script copy lets the inconsistency survive. Tests assert two groups, both load-bearing: the hydrogens come back (right species) AND the docked heavy atoms do not move (still the pose that was docked). MUTATION-TESTED, 3/3 killed. * The independence correction. M12/M13/M16 read "p > 0.05" as "the axes are independent" and built the exact sqrt(n) = 10x factor on it. At n=15 the CI on rho is [-0.682, +0.290] -- it does not exclude a strong correlation. Re-derived at the CI edge most favourable to selection, selection is still 7.3x worse than averaging vs 10.0x at rho=0, so the RANKING is robust and only the exact factor was conditional. Quoted as 7-10x throughout. * The numpy.bool_ latent bug: `informative = ps < 0.05` then `if informative is False:` never fired, because np.bool_(False) is not False. The script printed no verdict at all for its entire life. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mgoldey
added a commit
that referenced
this pull request
Sep 19, 2026
Rebased onto main as a clean set: the probe scripts, the RESULTS.md corrections, and tools/docking/united_atom.py. The old branch also carried a stale scripts/complexity_baseline.json that would have reverted #104, so this takes only the files that belong here. Contents: * run_docked_pose_scatter.py (M12), run_scorer_pose_sensitivity.py (M14), run_xtb_vs_dft_tracking.py (M16). * tools/docking/united_atom.restore_hydrogens, shared rather than copied. PDBQT is UNITED-ATOM: a docked danuglipron pose has 42 atoms and 263 electrons where the real molecule has 71 and 292. `embed_ligand_from_coords` REFUSES that (263 electrons at multiplicity 1 implies a non-integer n_alpha); `pose_fit` ACCEPTS IT SILENTLY and returns a normal-looking number for a molecule missing 29 hydrogens. M14 fixed it inside its own probe, M12's scatter did not -- which is how a per-script copy lets the inconsistency survive. Tests assert two groups, both load-bearing: the hydrogens come back (right species) AND the docked heavy atoms do not move (still the pose that was docked). MUTATION-TESTED, 3/3 killed. * The independence correction. M12/M13/M16 read "p > 0.05" as "the axes are independent" and built the exact sqrt(n) = 10x factor on it. At n=15 the CI on rho is [-0.682, +0.290] -- it does not exclude a strong correlation. Re-derived at the CI edge most favourable to selection, selection is still 7.3x worse than averaging vs 10.0x at rho=0, so the RANKING is robust and only the exact factor was conditional. Quoted as 7-10x throughout. * The numpy.bool_ latent bug: `informative = ps < 0.05` then `if informative is False:` never fired, because np.bool_(False) is not False. The script printed no verdict at all for its entire life. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
The bug
Every key in
scripts/complexity_baseline.jsonwas an absolute path with the leading slash removed:rust-code-analysis-climirrors each input's absolute path into its output tree, so the script'sos.path.relpath(fpath, tmp)recovers the absolute path, not a repo-relative one. All 6104 keys carried one checkout's directory.Two consequences
1. The gate is unusable in any git worktree. No key matches, so every function reads as NEW and is judged only against the bare
NEW_FUNCTION_CC_CEILINGinstead of its own recorded history. It fails on changes that touch no Rust at all — this was found by a commit whose entire diff was one.config/nextest.tomlfile. Most work in this repo happens in worktrees (there are ~15), so the gate has effectively been off for them.2. It was masking a real regression. Because nothing matched, the gate never compared a function against its own history:
Both grew in #88 (TRAH/AURORA-SCF). The script's own header still says
solve_rhf, CC=134, true when written.What this PR does
ROOTfrom the recovered path, so keys arecrates/...and portable across worktrees and machines (same reasoning as the repo's never commit machine-specific values convention).Verification
MUTATION TESTED — lowering
solve_rhf's baseline entry to CC=10:so the restored gate fires rather than passing vacuously.
Why its own PR
Four open branches (#99, #100, #101, and the viz branch) each hit this spurious failure and each would otherwise need its own cherry-pick and re-baseline. Landing it once unblocks all of them.
🤖 Generated with Claude Code
Summary by CodeRabbit