Eleven implementations of "is this path a test?" disagree on 5 of 11 paths, and none reads the is_test the ingestion already stored

[milaneliasde]

Nineteen places decide "is this path a test?" from the path string, under six different names, over seventeen separate pattern definitions. They disagree with each other, three of them classify production code as tests, and sixteen of them re-derive an answer the ingestion already computed and stored.

This is not a tidiness issue: three call sites return wrong answers for real paths, and two of the disagreements change what the health engine and the search rank.

Version: 0.36.0, verified against a clean uv pip install repowise==0.36.0 in an isolated venv.

Where the question is answered

File	Function
core/ingestion/traverser.py:674	_is_test_file(rel_path, filename) — this is the one that sets FileInfo.is_test (traverser.py:534)
core/analysis/health/engine.py:88	_is_test_file(rel_path)
core/analysis/communities.py:395	_is_test_file(path)
core/analysis/health/coverage/detector.py:115	is_test_file(rel_path, source)
core/generation/layers.py:104,119	_is_test_file_name + _is_test_dir_path, combined in infer_layer
core/analysis/health/refactoring/split_file.py:112	_is_test_path(path)
core/analysis/health/refactoring/move_method.py:55	_is_test_path(path)
core/analysis/health/refactoring/extract_helper.py:70	_is_test_path(path)
core/analysis/health/biomarkers/hidden_coupling.py:71	_is_test_path(path)
server/mcp_server/_answer_pipeline.py:201	_is_test_path(target_path)
server/mcp_server/tool_search_symbols.py:43	_is_test_path(path)
server/mcp_server/tool_search.py:366	inline over its own _TEST_PATH_TOKENS

_TEST_PATH_TOKENS is defined three times, byte-identical, in _answer_pipeline.py:194,
tool_search.py:331 and tool_search_symbols.py:35.

The full set

Grepping for _is_test_* finds eleven. Grepping for the behaviour — any function or expression
that classifies a path by test tokens — finds nineteen, spread over six names:

Name	Where
_is_test_file	ingestion/traverser.py:674 (sets FileInfo.is_test), analysis/health/engine.py:88, analysis/communities.py:395
is_test_file	analysis/health/coverage/detector.py:115
_is_test_file_name / _is_test_dir_path	generation/layers.py:104,119, combined in infer_layer
_is_test_path	health/refactoring/{split_file,move_method,extract_helper}.py, health/biomarkers/hidden_coupling.py, mcp_server/_answer_pipeline.py, mcp_server/tool_search_symbols.py
_looks_like_test_path	health/biomarkers/coverage_gap.py:74, health/biomarkers/coverage_gradient.py:46
is_test_path	ingestion/git_indexer/fix_shape.py:97, workspace/extractors/base.py:42

Four more decide inline, without a named predicate: mcp_server/tool_search.py:352
(_classify_hit_kind), server/routers/stats.py:545, cli/ui/repo_scanner.py:91,
generation/onboarding/subkinds/development_guide.py:81,117.

Seventeen separate pattern definitions back them: _TEST_PATH_TOKENS (×3, byte-identical),
_TEST_PATH_RE (×2, different), _TEST_DIR_SEGMENTS (×2, different), plus _TEST_DIRS,
_TEST_FILE_RE, _TEST_FILE_PATTERNS, _TEST_DIR_TOKENS, _TEST_DIR_FRAGMENTS,
_TEST_SUFFIXES, _TEST_BASENAME_PATTERNS, _TEST_FILE_PREFIXES, _TEST_FILE_SUFFIXES,
_TEST_PATH_FRAGMENTS, _TEST_PATTERNS, _TEST_ROOTS.

Worth noting: two of them are already called is_test_path, public, in
ingestion/git_indexer/ and workspace/extractors/. workspace/extractors/base.py is also the
only one that matches with fnmatch on the basename and on path segments rather than on
substrings — it gets the cases below right.

What they answer

Nine are importable and were run over the same paths. layers is called through its real entry
point infer_layer(path) == "Test", not through its private halves.

path	tool_search	answer_pipe	split_file	move_method	extract_helper	hidden_coupling	engine	communities	infer_layer
tests/app/tests.py	no	no	yes	yes	no	yes	no	yes	yes
spec/models/user_spec.rb	no	no	no	no	no	no	no	no	yes
conftest.py	no	no	yes	yes	no	no	no	yes	yes
src/latest/api.py	no	no	no	no	no	no	no	yes	no
protest/main.py	no	no	no	no	no	no	no	yes	no
src/test/java/Foo.java	yes	yes	yes	yes	yes	yes	yes	yes	yes
src/__tests__/x.js	yes	yes	yes	yes	yes	yes	yes	yes	yes
app/user_test.go	yes	yes	yes	yes	yes	yes	yes	yes	yes
src/Button.spec.ts	yes	yes	yes	yes	yes	yes	yes	yes	yes
src/testing/helpers.py	no	no	no	no	no	no	no	no	no
lib/contest.py	no	no	no	no	no	no	no	no	no

Disagreement on 5 of 11.

Findings

1. Three places classify production code as tests

_TEST_PATH_RE = re.compile(
    r"(test[s_/]|_test\.|\.test\.|\.spec\.|__tests__|conftest|fixture[s]?[/.])",
    re.IGNORECASE,
)

test[s_/] is meant to catch test/ and tests/, but it is not anchored to a path segment, so it
also fires on any segment ending in "test" followed by a separator. src/latest/api.py and
protest/main.py are classified as test files and are pulled out of community assignment
(_assign_tests_to_communities).

The same substring check appears in two more places, with the same effect:

cli/ui/repo_scanner.py:91                 any(p in full_rel for p in _TEST_PATTERNS)
generation/.../development_guide.py:81    any(seg in path for seg in _TEST_ROOTS)

Verified against 0.36.0: protest/main.py, src/latest/api.py and contest/x.py are counted as
tests by all three. In repo_scanner that inflates the test-file count shown during init; in
development_guide it drops those files out of the generated guidance.

2. tests/app/tests.py — five say test, four say not

The layout Django's own tutorial produces. It is skipped by three refactoring detectors and by
community assignment, but returned as an ordinary hit by both MCP search paths and not excluded by
the health engine.

Two of the implementations state in their own docstrings that they are meant to match the others:

Conservative test-file check (mirrors the other detectors). — split_file.py:113

Conservative test-file check (mirrors the engine's ``_is_test_file``). Kept local so the detector stays self-contained — extract_helper.py:71

extract_helper.py answers no for this path; split_file.py answers yes. The copies have drifted
apart from the thing they claim to mirror.

3. RSpec layout is recognised by exactly one of nine

spec/models/user_spec.rb is only classified as a test by infer_layer. RSpec's convention is
spec/**/*_spec.rb, so in Ruby projects the health engine, both refactoring detectors, the
biomarker and both MCP search paths treat specs as production code.

4. conftest.py — defensible either way, argued nowhere

Four say test, five say not. Strictly, pytest does not collect conftest.py as a test module: it is a
per-directory plugin providing fixtures. For a refactoring detector, treating test infrastructure
like a test is equally defensible. Both readings are reasonable; what is missing is that either is
stated anywhere, so the same file is treated differently depending on which tool asks.

5. Sixteen of them re-derive a decision that is already stored

traverser.py:534 computes is_test per file during ingestion and stores it on FileInfo
(core/ingestion/models.py:132). It is carried into the graph (graph/builder.py:187) and
propagated onto production files reached by tests (graph/_edges.py:102).

That flag is the established mechanism, not an unused field. Roughly a dozen consumers read it:
analysis/dead_code/analyzer.py (three sites), analysis/knowledge_graph.py,
analysis/pr_blast.py, ingestion/framework_edges/pytest_edges.py,
generation/page_generator/helpers.py, generation/context/assembler.py, pipeline/persist.py,
persistence/crud/graph.py.

The split is between subsystems that hold a graph node or a FileInfo and use the flag, and
subsystems that hold only a path string and re-derive. Of the nineteen path-based sites, three do
use the stored value:

mcp_server/tool_search_symbols.py:117   (gnode is not None and gnode.is_test) or _is_test_path(row.file_path)
mcp_server/tool_search_symbols.py:241   is_test = (g is not None and g.is_test) or _is_test_path(row.file_path)
server/routers/stats.py:537             GraphNode.is_test.is_(False)

The remaining sixteen do not.

6. The stored flag has a real gap, and one call site documents it

stats.py reads the flag and applies its own pattern, and says why:

# catches conftest, fixtures, and spec files the is_test flag misses. — stats.py:79

Measured against 0.36.0, the comment is partly outdated and partly correct. conftest.py,
spec/spec_helper.rb, spec/models/user_spec.rb and tests/fixtures/sample.json are all caught by
the flag today. These are not:

path	traverser._is_test_file
fixtures/data.yml	no
src/__fixtures__/x.ts	no
testdata/golden.json	no

Fixture and golden-data directories outside a test tree are missed. This matters for the shape of a
fix: "just read the stored flag" would make those call sites worse, not better. The order has to be
the other way round — close the gap in the one authoritative computation first, then migrate the
callers onto it.

stats.py is the counter-example to the pattern in this report, not an instance of it: it uses the
stored decision, knows where it falls short, and says so in the code.

Why it matters

A single file is a test and not a test at the same time, depending on which subsystem asks. Health
scores, dead-code findings, refactoring suggestions, community assignment and search ranking each
use their own answer. The three verbatim copies of _TEST_PATH_TOKENS mean a fix to one search path
silently leaves the other two behind.

The existing answer to this does not scale

tests/unit/health/test_mts_cts_test_classification.py shows how this class of problem was handled
before: ".mts/.cts test files must be classified as tests across health logic (#288)". It imports
four implementations by name and asserts each one separately:

from ...biomarkers.coverage_gap import _looks_like_test_path
from ...biomarkers.hidden_coupling import _is_test_path as _coupling_is_test_path
from ...coverage import is_test_file
from ...engine import _is_test_file as _engine_is_test_file

(That file also surfaces a twelfth implementation I had not listed: coverage_gap._looks_like_test_path.)

The cost of that approach grows with every incident and every copy:

• it pins one input class (.mts/.cts) and says nothing about the five disagreements above;
• a thirteenth implementation is not covered by it, so it may be wrong from the day it is written;
• it makes the copies harder to remove, because the test now names four of them explicitly.

A single source inverts that. Adding an ecosystem becomes one row in the pattern table, and if the
test is parametrised over that table rather than over the call sites, the new row brings its own
coverage with it — the property that makes table-driven tests worth having: "once the test function
exists, adding a new scenario is a single row of data".

The "should there be a thirteenth copy" question is then answerable by the build rather than by
review. A rule that fails when a new private _is_test_* helper appears outside the shared module is
an architecture fitness function in the usual sense — "an objective, automated check that a given
architectural characteristic still holds … converts a rule you hope people follow into a rule the
build enforces" — and in Python it is what import-linter is normally used for.

Suggested shape

One decision, one helper.

1. First close the gap in traverser._is_test_file (fixture and golden-data directories outside a
   test tree), so the stored flag can carry the weight. Then, where a FileInfo or a graph node is
   in hand, read is_test — as a dozen consumers already do.
2. For the path-only callers (the MCP tools), one shared function, in one place.
3. Its patterns as data, grouped by ecosystem, with the convention named in a comment:

TEST_PATTERNS: dict[str, tuple[str, ...]] = {
    "maven_gradle": ("**/src/test/**", "**/src/androidTest/**"),
    "jest":         ("**/__tests__/**", "**/*.test.*", "**/*.spec.*"),
    "pytest":       ("**/test_*.py", "**/*_test.py", "**/tests/**", "**/test/**"),
    "rspec":        ("**/spec/**/*_spec.rb",),
    "go":           ("**/*_test.go",),
}

4. Match on path segments and globs, never on substrings — ** crosses directory boundaries, *
   does not, which is exactly the property finding 1 needs.
5. Give conftest.py its own flag (test support), so refactoring detectors can skip it without
   search filing it away as a test.
6. Where a subsystem deliberately deviates, say so in the code — as extract_helper.py tried to,
   but with a deviation the comment actually describes.

The migration can be done one call site at a time: add the module, convert one site, diff its old
answer against the new one over a corpus of paths. Where the answer changes, it is one of the
findings above. The three copies of _TEST_PATH_TOKENS fall out on the way.

Note on method

layers.py is the one implementation where the answer depends on which entry point you call, and it
is worth flagging for anyone reproducing this. Calling the private _is_test_file_name in isolation
makes it look as though layers.py misses the Maven and Jest layouts. It does not: that helper is
documented as looking at the filename alone, and infer_layer combines it with _is_test_dir_path,
which handles the directory conventions. The table above calls infer_layer(path) == "Test" for that
reason.

What was measured and what was not

Measured against a clean uv pip install repowise==0.36.0 in an isolated venv: every file:line
reference in this report (31, checked automatically against the source); the comparison table (the
listed predicates were imported and executed); the substring behaviour of the three affected sites;
the gap in the stored flag; which sites read the flag.

Not measured: coverage/detector.is_test_file also takes the file contents, so a path-only helper
cannot replace it as-is. Whether any of the nineteen needs a different answer for its own purpose
was not assessed per call site — the report shows that they differ and that three are wrong, not
that all nineteen must return the same thing. Two earlier drafts of this report contained mistakes
of exactly that kind, both found by executing the code rather than reading it.

[RaghavChamadiya]

Thank you for putting this much work into the report, the reproduction table and the note on layers.py entry points made it verifiable end to end instead of a judgement call.

Taking this on. #1089 is moving in parallel, so this one will start after that arc closes. I reproduced the table against current main and the core claim holds, so what follows is mostly notes on what changed under verification, plus one correction.

The count is higher than eleven

Two more re-derive the same question and are not in the list:

File	Function
core/analysis/health/biomarkers/coverage_gradient.py:46	_looks_like_test_path(path)
server/routers/stats.py:154	_TEST_PATH_RE, used at :703

stats.py is a fourth verbatim-ish copy of the communities.py regex, so finding 1's unanchored
test[s_/] bug is in the server too, not only in analysis.

coverage_gradient is the argument for doing this

coverage_gradient._looks_like_test_path is a near-copy of coverage_gap._looks_like_test_path
that never received the #288 fix:

path                  coverage_gap  coverage_gradient
src/foo.test.mts      True          False
src/foo.spec.cts      True          False
src/foo.test.ts       True          True

The #288 test imports coverage_gap, hidden_coupling, coverage.is_test_file and engine. It does
not import coverage_gradient, so the fix landed on four copies out of five and the fifth has been
wrong ever since, in the specific input class the test exists to protect. The "a thirteenth
implementation is not covered by it, so it may be wrong from the day it is written" argument is not
hypothetical here, it already happened and the pinning test reported green throughout.

Finding 5 is overstated, and the exception is useful

tool_search_symbols.py does read the stored decision:

# :117
if (gnode is not None and gnode.is_test) or _is_test_path(row.file_path):
# :241
is_test = (g is not None and g.is_test) or _is_test_path(row.file_path)

The grep patterns in the issue (file_info.is_test, ["is_test"], .get("is_test") do not cover
attribute access on a graph node, which is why this came back clean. It matters because that line is
already the shape being proposed (stored flag first, path fallback only when there is no node), so it
is the migration template rather than a violation. The other search path, tool_search.py:366, has
the node available and does not use it.

Two stored flags, not one

FileInfo.is_test and the graph node's is_test are different predicates. graph/builder.py:187
copies the file flag in, then graph/_edges.py:102 widens it:

if e.hint_source and e.hint_source.endswith(":test"):
    self._graph.nodes[e.source]["is_test"] = True

So the graph flag is a superset that also covers the source of any dynamic edge whose hint came from a
test. "Read the stored decision" therefore has to name which one per call site. A refactoring detector
skipping files should probably want the narrow file flag; search de-ranking probably wants the wide
one. Worth settling before the first conversion, since both are reachable from most call sites and
they differ silently.

On the pattern table

I would rather lift traverser._is_test_file than write a new table. It is the only implementation
that is already segment-anchored (it splits on / and compares whole segments, so finding 1 cannot
happen there), and it takes its conventions from the language registry instead of a local literal,
which is where a new ecosystem already belongs. A fresh TEST_PATTERNS dict would be a second place
to declare Go's _test.go next to the registry that declares it today.

The glob-versus-segment point stands either way, it is just that the anchoring already exists in the
implementation that feeds the stored flag.

stats.py shows why point 6 needs to be stronger

stats.py:153 already documents its deviation:

# Test-path convention shared with analysis (communities / execution_flows):
# catches conftest, fixtures, and spec files the `is_test` flag misses.

That comment is accurate about intent and the regex is still wrong, because a local pattern is where
the bug lives regardless of what the comment above it says. So the conftest/fixtures reading needs to
be a named option on the shared helper (point 5's separate flag, reachable as a parameter) rather than
a documented licence to keep a local regex. Deviation by argument, not by copy.

Corrections to finding 1

manifest/ does not fire, it ends in "fest". Confirmed hits are latest/, protest/, contest/,
and directory form only: lib/contest.py is correctly negative because the character after "test" is
., not one of [s_/]. Consistent with the table in the issue, just narrowing the list of
real-world names.

Sequencing

Agreed on one call site at a time with an old-versus-new diff over a path corpus. Two additions:

1. Land the corpus and the parametrised test first, over the pattern data rather than over the call
   sites, and delete test_mts_cts_test_classification.py at the end rather than extending it. It
   names four implementations explicitly and is the thing making them harder to remove.
2. Do coverage_gradient early and as its own commit, since that one is a live bug rather than a
   consistency cleanup and should be reviewable on its own.

The import-linter rule is the right closing move. Contract shape is a forbidden contract from every
analysis/server module to anything defining a private _is_test_*, which in practice means the rule
has to be "these modules may not define it", so it likely wants a small custom check rather than
stock import-linter contracts. I will confirm which when I get there.