feat: graph-based targeted test selection for health checker (#1451) - #1454
feat: graph-based targeted test selection for health checker (#1451)#1454colehurwitz wants to merge 4 commits into
Conversation
Add reverse-import BFS over graph.json to select only tests affected by changed files, reducing health-check gate time from 8-10 min to 1-3 min for focused changes. Falls back to full suite on any ambiguity (stale graph, conftest changes, unknown files, >80% fan-out). - factory/graph.py: find_dependent_tests() with conservative None fallback - factory/eval/languages/python.py: optional test_paths parameter - factory/eval/runner.py + cli: --targeted flag with structlog auditing - factory/agents/prompts/health_checker.md: use factory eval --targeted - 21 new tests covering BFS, fan-out, triggers, and path threading Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sentrux Quality ReportAbsoluteDiff (vs base branch) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1454 +/- ##
==========================================
+ Coverage 82.33% 82.36% +0.03%
==========================================
Files 222 224 +2
Lines 24555 24904 +349
Branches 3925 4005 +80
==========================================
+ Hits 20217 20512 +295
- Misses 3405 3445 +40
- Partials 933 947 +14 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@ceo-review |
There was a problem hiding this comment.
✅ Factory Review: KEEP
Verdict: KEEP
Reason: QA: CLEAN — 601 tests pass, composite 0.9631, code review 7/7 PASS, adversarial 13/13 PASS, 0 issues
QA Analysis
Adversarial_Tester Agent Output
- timestamp: 2026-09-04T18:45:19Z
- exit_code: 0
Adversarial QA Verdict: PASS ✅
Evidence Summary
| # | What I Tested | How | Result |
|---|---|---|---|
| Smoke | Project-wide smoke test | pytest tests/test_models.py tests/test_guards.py tests/test_runners.py |
165 passed |
| 1 | Full targeted test suite | pytest tests/test_targeted_tests.py -v |
35/35 passed |
| 2 | Import path | python -c 'from factory.graph import find_dependent_tests' |
Clean import, correct signature |
| 3 | CLI flag | factory eval --help |
--targeted present with correct help text |
| 4 | Functional: reverse BFS | Real graph.json on disk, multiple scenarios | Correct transitive + direct results |
| 5 | Edge: no graph.json | Called with empty temp dir | Returns None (graceful fallback) |
| 6 | Edge: corrupt graph.json | Invalid JSON content | Returns None with warning (no crash) |
| 7 | Edge: "links" key format | Alternative graph format | Works correctly |
| 8-9 | Backward compat | Without --targeted, without attr |
_compute_targeted_test_paths never called |
| 10 | Call chain threading | inspect.signature on 5 functions |
test_paths param present everywhere |
| 11 | Fan-out threshold | 80% boundary vs 90% | Correct > behavior (80% passes, 90% triggers) |
| 12 | Real git repo | _compute_targeted_test_paths(Path('.')) |
Graceful None in detached HEAD |
| 13 | Signature guard | hygiene.py introspection | Correctly omits test_paths for older evaluators |
One minor observation (not a blocker): Root-level conftest.py without a directory prefix won't match **/conftest.py in fnmatch. This is a low-risk gap with multiple safety nets.
The feature is solid. Every failure path returns None → full suite fallback. No crashes, no regressions.
⚠ CEO IDENTITY RE-ANCHOR (Sacred Rule 8)
You are the Factory CEO. You orchestrate, delegate, and decide. You do NOT implement.
If you are about to write code, run tests, do research, or fix bugs — STOP and spawn the appropriate agent.
Re-read your Permitted/Forbidden Actions lists in the Identity section above.
Posted by Factory CEO
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…t scan Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@ceo-review |
|
@lambdabaa a small PR to lower the amount of testing for health checker based on what is affected by the PR. This step is extremely slow and times out a lot. |
There was a problem hiding this comment.
✅ Factory Review: KEEP
Verdict: KEEP
Reason: QA: CLEAN — 601 tests pass, composite 0.963, all 7 code review categories PASS, all 8 adversarial acceptance criteria verified. No critical issues.
QA Analysis
Adversarial QA Report — PR #1454: Graph-Based Targeted Test Selection
Project type: Library (CLI + eval framework)
Date: 2026-09-04
Verdict: ✅ PASS
Smoke Test
Command: uv run pytest tests/test_models.py tests/test_guards.py tests/test_runners.py -x -q --tb=short -k 'not (BobAuth or preflight_error_unchanged)'
Result: 165 passed in 4.48s
Status: ✅ PASS
Test Plan (derived from PR scope)
| # | Criterion | Status |
|---|---|---|
| A | All 39 unit tests pass | ✅ VERIFIED |
| B | find_dependent_tests works with real temp graph | ✅ VERIFIED |
| C | Edge cases: stale/conftest/unknown/fan-out | ✅ VERIFIED |
| D | Grep-based inline import scan works | ✅ VERIFIED |
| E | inspect.signature guard works | ✅ VERIFIED |
| F | Adversarial inputs don't crash | ✅ VERIFIED |
| G | --targeted CLI flag exists and parses | ✅ VERIFIED |
| H | test_paths threads through full chain | ✅ VERIFIED |
Test A: All 39 unit tests pass
Command: uv run pytest tests/test_targeted_tests.py -v
Output: 39 passed in 0.29s — all tests green
Status: ✅ VERIFIED
Test B: find_dependent_tests with real temp graph
Command: Python script creating temp graph.json and calling find_dependent_tests directly
Evidence:
- Change
src/a.py→ found['tests/test_a.py', 'tests/test_b.py'](transitive BFS) - Change
src/b.py→ found['tests/test_b.py']only (direct import) - Change
tests/test_a.py→ found['tests/test_a.py'](self-inclusion)
Status: ✅ VERIFIED
Test C: Edge cases
All tested via direct Python calls with mocked staleness:
| Edge Case | Input | Expected | Actual | Status |
|---|---|---|---|---|
| Stale graph | is_graph_stale=True |
None | None | ✅ |
| Unknown staleness | is_graph_stale=None |
None | None | ✅ |
| conftest.py | ['tests/conftest.py'] |
None | None | ✅ |
| Nested conftest | ['tests/sub/conftest.py'] |
None | None | ✅ |
__init__.py |
['factory/__init__.py'] |
None | None | ✅ |
| Unknown .py file | ['src/unknown.py'] |
None | None | ✅ |
| Fan-out 100% | 10/10 tests depend | None | None | ✅ |
| Empty list | [] |
None | None | ✅ |
| pyproject.toml | ['pyproject.toml'] |
None | None | ✅ |
| CI workflow | ['.github/workflows/ci.yml'] |
None | None | ✅ |
| Fan-out exactly 80% | 8/10 tests depend | set(8) | set(8) | ✅ |
| Fan-out 90% | 9/10 tests depend | None | None | ✅ |
Status: ✅ VERIFIED
Test D: Grep-based inline import scan
Command: Python script creating test files with inline imports on disk
Evidence:
- Test file with
from factory.runners.claude import runinside function body → grep found it - Test file without matching import → grep correctly returned empty
- Top-level file (
setup.py, parts < 2) → grep skipped correctly
Status: ✅ VERIFIED
Test E: inspect.signature guard
Command: Python scripts testing both old-style and new-style evaluators
Evidence:
PythonEvaluator.run_tests_with_coveragehastest_pathsparam → ✅PythonEvaluator.run_testshastest_pathsparam → ✅_collect_test_and_coveragesource containsinspect.signature→ ✅- Old evaluator without
test_pathsparam → guard omits it, no crash → ✅ - New evaluator with
test_pathsparam → guard passes it through → ✅
Status: ✅ VERIFIED
Test F: Adversarial inputs
| Test | Input | Expected | Actual | Status |
|---|---|---|---|---|
| Empty list | [] |
None | None | ✅ |
| File with hyphen | src/my-module.py |
set() | set() | ✅ |
| File with space | src/my module.py |
set() | set() | ✅ |
| 10-hop transitive chain | mod_0 ← ... ← mod_9 ← test_deep | {tests/test_deep.py} |
{tests/test_deep.py} |
✅ |
| Corrupted JSON | {invalid json!!!} |
None | None | ✅ |
| Empty nodes/edges | {'nodes':[], 'edges':[]} |
None | None | ✅ |
| Non-python file | Makefile |
None | None | ✅ |
| Mixed full-suite trigger | ['src/a.py', 'tests/conftest.py'] |
None | None | ✅ |
links key alias |
graph with links not edges |
set with results | set with results | ✅ |
Status: ✅ VERIFIED
Test G: --targeted CLI flag
Command: parser.parse_args(['eval', '/tmp/fake', '--targeted'])
Evidence:
- With
--targeted:args.targeted = True→ ✅ - Without flag:
args.targeted = False→ ✅ - Flag registered in
factory/cli/_parser_groups.pywithaction="store_true"→ ✅
Status: ✅ VERIFIED
Test H: test_paths threading through full chain
Command: inspect.signature() on each function in the chain
Evidence:
cmd_eval→ readsargs.targeted, calls_compute_targeted_test_paths, passes torun_eval→ ✅run_evalhastest_pathsparam → ✅compute_hygiene_resultshastest_pathsparam → ✅_collect_test_and_coveragehastest_pathsparam → ✅PythonEvaluator.run_tests_with_coverageappends paths to pytest command after-qflag → ✅PythonEvaluator.run_testsdelegates torun_tests_with_coveragewithtest_paths→ ✅
Status: ✅ VERIFIED
Process Cleanup
No servers started. No tmux sessions created. No orphaned processes.
Adversarial Verdict: ✅ PASS
All 8 acceptance criteria verified with concrete evidence. The feature:
- Correctly performs reverse BFS over import graphs to find affected tests
- Correctly falls back to full suite on stale graphs, conftest changes, unknown files, and high fan-out
- Catches inline imports via grep that graphify misses
- Guards against evaluators that lack the test_paths parameter via inspect.signature
- Threads test_paths through the full CLI → runner → hygiene → evaluator chain
- Handles adversarial inputs (empty lists, special chars, deep chains, corrupted JSON) gracefully without crashes
Posted by Factory CEO
Summary
find_dependent_tests()tofactory/graph.py— reverse-import BFS over the existinggraph.jsonto compute which test files are transitively affected by changed source filestest_pathsparameter throughPythonEvaluator→compute_hygiene_results()→run_eval()so pytest runs only the affected subsetfactory eval --targetedCLI flag that computes changed files, resolves dependencies via the graph, and falls back to full suite when selection cannot be trusted--targetedby defaultSafety design
find_dependent_tests()returnsNone(not empty set) on any ambiguity → callers always fall back to full suiteconftest.py,__init__.py,pyproject.toml, CI filescheck_qa_execution()is unaffected (checks event presence, not pytest scope)Inline import coverage
Adversarial testing found 56% of test files use inline imports invisible to graphifyy's static graph. The grep-based scan (runs after the reverse-BFS) catches these by searching test files for the changed module's dotted import path. Validated on
factory/runners/claude.py— previously missedtest_ceo_message_events.pyandtest_verification.py, now found.Files changed
factory/graph.py—find_dependent_tests()with reverse BFS + grep-based inline import scanfactory/eval/languages/python.py—test_pathsparam onrun_tests_with_coverage()/run_tests()factory/eval/hygiene.py— threadtest_pathsthrough hygiene pipelinefactory/eval/runner.py— threadtest_pathsthroughrun_eval()factory/cli/eval_cmds.py—--targetedflag +_compute_targeted_test_paths()factory/cli/_parser_groups.py— argparse flag registrationfactory/agents/prompts/health_checker.md— one-line prompt updatetests/test_targeted_tests.py— 39 tests covering BFS, CLI integration, grep scanTest plan
Closes #1451
🤖 Generated with Claude Code
https://claude.ai/code/session_01YYiei7JyDnjMrt2Dg6jkJu