[jonahgabriel]

OMN-16917 follow-up — the selection predicate and the sizing predicate are now the same predicate

Follow-up to omnibase_core#1614 (merged to dev as 22adaa83), landing a valid CodeRabbit review finding on that PR that was orphaned: the repo's auto-squash bot merged #1614 on its first commit while the second commit's governed full-suite pre-push was still running, so the fix never reached dev. This PR is that commit, cherry-picked onto post-merge dev.

The defect

TEST_FILE_PATTERNS mirrors pytest's python_files and holds two patterns:

TEST_FILE_PATTERNS = ("test_*.py", "*_test.py")

is_test_file_name honours both. _contains_collectable_test therefore admits a directory on either pattern — and ci_contract_test_paths (added in #1614) selects tests/ci/ on exactly that predicate.

But _count_test_files globbed only one half:

total += sum(1 for _ in target.rglob("test_*.py"))

So a directory could be selected on the strength of files the volume counter then did not count. The consequence runs through _split_count_for → VOLUME_THRESHOLD_FILES / VOLUME_TARGET_FILES_PER_SPLIT: undercounted volume, too few matrix splits, each split carrying more than the sizing intended. A selection whose own justification was invisible to its sizing.

The change

_count_test_files now counts every name is_test_file_name accepts, so the predicate that decides whether a directory is selected and the predicate that decides how big that selection is are the same predicate. One function, one source of truth, no second copy of the pattern list to drift.

Behavior on the current tree

Unchanged. The live tree holds 0 *_test.py files against 1,586 test_*.py, so no selection is sized differently today. This closes a latent divergence rather than reacting to a live miscount — which is precisely why it needed a test that does not depend on the live tree's population.

test_split_count_counts_every_configured_test_filename_pattern asserts the counter directly, against a synthetic tree holding one file of each pattern plus a non-test helper:

tests/suite/test_prefix_style.py      -> counted
tests/suite/suffix_style_test.py      -> counted   (would have been missed)
tests/suite/helper.py                 -> not counted

so the invariant holds regardless of which patterns happen to be populated in the repo later.

Readback unchanged

Same real OMN-16625 5-file diff that #1614 was measured on:

split_count: 5   is_full_suite: false   tests/unit/ absent

Identical to the post-merge readback on 22adaa83. This PR narrows nothing and widens nothing on today's tree.

No bypass

No [skip-* token, no ENABLE_SMART_TESTS=off, no PREPUSH_FULL_SUITE, no new environment variable. The pre-existing knobs are untouched.

dod_evidence

• uv run pytest tests/unit/scripts/ci/test_detect_test_paths_ci_contract_omn16917.py → 16 passed (15 from fix(OMN-16917): apply the OMN-16745 CI-contract ruling to core's own import-graph selector #1614 plus the new counter test).
• Live selector readback on the real OMN-16625 file list after this change: split_count: 5, is_full_suite=false, tests/unit/ absent — byte-identical to the merged 22adaa83 readback.
• Governed pre-push selector on this branch: correctly escalated is_full_suite=true, full_suite_reason=test_infrastructure (this PR changes scripts/ci/detect_test_paths.py, a test_infrastructure_paths entry) → the hook ran the full suite and allowed the push. No knob set, no override grant, no --no-verify.
• uv run ruff format --check / uv run ruff check on both changed files → clean. Full pre-commit ran as part of the push.
• Review thread on fix(OMN-16917): apply the OMN-16745 CI-contract ruling to core's own import-graph selector #1614 replied to and resolved with this reasoning.

Ticket: OMN-16917

Evidence-Ticket: OMN-16917
Evidence-Source: OCC#7544

Summary by CodeRabbit

• Bug Fixes

  • Improved test discovery for directory selections by recognizing both supported pytest filename patterns.
  • Non-test files are now excluded from test counts as expected.

• Tests

  • Added coverage to verify accurate counting across multiple test filename patterns.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7cb4869b-15dd-4207-a76a-60ffe5a4aecf

📥 Commits

Reviewing files that changed from the base of the PR and between 9b8b570 and 767b00e.

📒 Files selected for processing (2)

• scripts/ci/detect_test_paths.py
• tests/unit/scripts/ci/test_detect_test_paths_ci_contract_omn16917.py

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

---

📝 Walkthrough

Walkthrough

_count_test_files now counts both configured pytest filename patterns for directory selections. Tests cover prefix-style tests, suffix-style tests, and non-test helper files.

Changes

Test path counting

Layer / File(s)	Summary
Count all configured test patterns scripts/ci/detect_test_paths.py, tests/unit/scripts/ci/test_detect_test_paths_ci_contract_omn16917.py	_count_test_files now counts every filename accepted by is_test_file_name during directory traversal. Direct file selections remain counted directly. Tests validate both test patterns and exclude helper files.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 767b0

This localized change makes test-volume sizing count all configured test filename patterns and includes direct coverage for the previously missed pattern. No actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: counting every configured test filename pattern when sizing selector splits.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jonah/omn-16917-split-count-both-patterns

---

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