fix(hooks): run autobot-slm-backend as its own pytest invocation (#16069)

[mrveiss]

Refs #16069

Thinking Path

The reported symptom is a leak: autobot-slm-backend/conftest.py leaves 54 sys.modules keys installed while pytest works outside its directory, the guard blocks the push, and #15996 has been stuck behind it for hours. Two sessions looked at the conftest. It is 412 lines, its stubbing is deliberate and documented, and it is byte-identical to base — as is sys_modules_leak_baseline.txt. A guard was failing a push over unchanged code.

pytest.ini:64-73 says why:

#13084: autobot-backend/ and autobot-slm-backend/ each define top-level packages with identical names (api, services, user_management, middleware, migrations). Running ALL of testpaths in ONE pytest invocation makes whichever backend collects first bind those dotted names in sys.modules for the rest of the session, breaking the other backend's own tests. A full local/CI run MUST use two separate invocations.

CI obeys that. The pre-push hook did not — tools/git-hooks/pre-push:259 gathered every changed test into one $all_py_tests and ran a single pytest over it. A changeset touching both trees was therefore verified in a shape CI never runs and pytest.ini forbids, and the leak the guard reported is what that shape produces: the conftest stubs at import, so sqlalchemy, sqlalchemy.orm, sqlalchemy.ext.asyncio, models and models.database are still MagicMocks when repo_tests collects afterwards.

The guard was right, the conftest was innocent of this push, and the invocation was the defect.

What Changed

tools/git-hooks/pre-push partitions $all_py_tests on the anchored autobot-slm-backend/ prefix and runs each non-empty group as its own pytest, aggregating failure and timeout across them.

The common case is unchanged: a changeset inside one tree yields one empty group, which is skipped, so exactly one invocation runs. Only a changeset spanning both backends runs twice — the case that previously could not pass at all.

Failure semantics are preserved: any group failing sets EXIT_CODE=1 and prints its tail; any group hitting 124 routes to did_not_run, so a timeout still fails the push (#15985). A real failure outranks a timeout, since a timeout is "did not run" and a failure is information.

Verification

The guard runs the hook's own grep expressions, extracted from the file — a reimplementation would agree with itself and prove nothing, the same reason the #15985 tests execute the helper rather than grepping for its message.

case	slm_py_tests	other_py_tests
mixed (2 slm, 2 other)	the 2 slm paths	the 2 other paths
slm only	1	empty
other only	empty	2
repo_tests/fixtures/autobot-slm-backend/decoy_test.py + a real slm path	the real one only	the decoy

The decoy row is the one worth having: the prefix is anchored, and an unanchored matcher is exactly how the commit-trailer re-derivation reported 3,444 violations (RATCHET_BASELINES.md rule 5).

A fifth test is structural rather than behavioural, because the two assignments can be correct while a stray combined pytest $all_py_tests still runs beside them — and that combination would pass every behavioural test in the file.

A harness bug caught by re-deriving rather than by reading. The first version interpolated the path list with a Python repr, so bash received the two characters \n instead of newlines, every case collapsed to a single line, and the harness passed while testing nothing. It feeds stdin now. Recording it because it is the same shape as the defect above: a check that ran, reported success, and measured something other than what it named.

Model Used

Claude Opus 5 (1M context).

Correction, and then its correction — the claim stands, on different evidence

I asserted this unblocks #15996, then withdrew it, then restored it. The round trip is worth keeping because the withdrawal was a correct measurement of the wrong interval, which is subtler than being wrong.

The withdrawal. gh pr diff 15996 --name-only returns four files — three .claude/hooks/*.sh and one repo_tests/*_test.py — with no path under autobot-slm-backend/. Partition that and the slm group is empty, so the combined-session leak cannot arise. That reasoning is sound and the measurement is accurate.

Why it was the wrong interval. The hook does not diff the PR against base. tools/git-hooks/pre-push:112:

diff_range="${remote_sha}..${local_sha}"

That is the push range — what this push introduces relative to the remote branch head — not the net change against Dev_new_gui. The blocked branch carries a base merge, so relative to its remote head the push introduces everything base has gained since, including autobot-slm-backend/conftest.py itself and an slm test file. The hook's own running pytest on: line confirms it: autobot-slm-backend/slm/agent/health_collector_unit_states_16019_test.py alongside three repo_tests files the PR never touched.

So an slm path genuinely enters the target set, one invocation spans both trees, #13084 fires, and the conftest leaks. This partition is the fix for that, not an adjacent improvement.

Two ranges, both correct, answering different questions:

range	question	for the blocked branch
origin/Dev_new_gui...HEAD	what does my PR change?	4 files, no slm
remote_sha..local_sha	what does this push introduce?	the whole merge, slm included

Nothing in either output announces which question it answers, which is why substituting one for the other felt like a decisive check rather than a category error.

A consequence worth noting: the longer such a push stays blocked, the more base its range accumulates, and the likelier an slm path is in it. That is #16015's starvation loop in miniature — the block makes itself more likely to persist.

Single-issue rationale

This is one half of #16069 and deliberately does not close it. The other half — making the conftest stop leaking — is a refactor of shared SLM test infrastructure with live risk to that suite, and it carries an acceptance criterion this change cannot satisfy: every green previously obtained against the mocked sqlalchemy/models has to be re-earned, not assumed. There is one confirmed instance already (a test on #16027 that passed vacuously against the unspecced MagicMock), so the number found by intuition is a lower bound on the number that exist. Landing the invocation fix first unblocks the pushes that are stuck today without granting the leak any permanence, and without pretending the sweep has been done.

---

Second, independent reproduction — my own push, a different branch

Not the branch that motivated this. #15980 is a dependabot PR that needed a base merge to pick up a floors fix. The merge is clean, the parity divergence it was red for is resolved, and the push is refused:

1 file(s) above are NOT on repo_tests/sys_modules_leak_baseline.txt, so this run fails.
[pre-push FAIL] Push BLOCKED.

The two ranges, measured on that branch:

range	slm test files
remote_sha..local_sha — what the hook diffs	2
origin/Dev_new_gui...HEAD — what the PR changes	0 test files

$ git diff --name-only fd01f7af7..64f19c1bf | grep -E '_test\.py$' | sed 's|/.*||' | sort | uniq -c
      2 autobot-slm-backend
      3 repo_tests

Both trees in one target list, one pytest invocation, #13084 fires, the conftest's stubs are live while repo_tests collects, the leak guard refuses the push. Exactly the mechanism described above, on a branch with no relationship to the first case.

The base merge is what does it, which is why this will keep happening. Any branch behind base that merges to catch up pulls base's recent autobot-slm-backend/*_test.py files into its push range — and base gained several today. The condition is not rare and is not specific to one author.

Two PRs are now blocked on this: #15996 and #15980. Neither can push without --no-verify or AUTOBOT_SYSMODULES_GUARD=off, and both of those trade a real guard for a green, which is the wrong trade twice over — the guard is correct and the invocation is what is wrong.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 0793cc36-14eb-4021-a786-8fa194c019f3

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.