Skip to content

blastRadius applies the untested-risk boost to the wrong files: has_test_coverage is fed "is a test file", so a test file scores 1/1.2 the risk of an identical untested source #85

Description

@lyubomir-bozhinov

Severity: Mediumclass-C. Every number is returned, the shape is right, and the
"untested = riskier" adjustment is applied to the wrong files — the exact inverse of what the
function it feeds documents.

compute_blast_impact documents its fourth argument:

untested_boost = 1.2 when the file has no test coverage (untested = riskier), and 1.0 when it
does have coverage

and the shipped unit test spells the intended mapping out in its own comment:

// hops=1, pagerank=1.0, untested → 0.6 (medium)
// hops=1, pagerank=1.0, tested   → 0.5 (medium)

The value passed at blast_radius.rs:237 asks a different question:

let has_test = covered_test_files.contains(&path)
    || test_map
        .values()
        .any(|refs| refs.iter().any(|r| r.path == path));
let risk = compute_blast_impact(hops, &edge_type, file_pagerank, has_test);

Both disjuncts search the map's values"is path somebody's test file". "Does path have
test coverage"
is a lookup over its keys (test_map.contains_key(path)). So a test file is
scored as covered, and a source file with tests is scored as untested.

Measured

Fixture ~/qa-scratch/riskfix, built so the boost is the only thing that can differ. Every
dependent has exactly one outgoing Import edge to the seed and no incoming edges, so hops and
edge_weight are equal across all of them by construction.

Seed core.py, published ghcr.io/barnett-studios/cxpak:3.1.4:

path            risk                  hops  edge_type  category
dep1.py         0.1363638016949835    1     import     direct_dependents
dep2.py         0.1363638016949835    1     import     direct_dependents
dep4.py         0.1363638016949835    1     import     direct_dependents
tests/dep3.py   0.11363650141248624   1     import     test_files
CONTROL   risk(dep1) == risk(dep2)                bit-identical  -> pagerank is symmetric across
                                                                   these nodes, so a risk
                                                                   difference below is attributable
          risk(dep1) / risk(tests/dep3.py)        1.200000       -> exactly untested_boost

dep1.py, dep2.py and tests/dep3.py are byte-identical files in different directories. None of
the three has any test coverage. The one under tests/ is scored 1/1.2 as risky as the other two,
for no reason but its path.

Identical on both surfaces: cxpak/blastRadius over LSP and cxpak_graph op=blast_radius over MCP
return the same values to the last digit.

Why it matters beyond the decimal

The buckets are hard thresholds — >= 0.7 high, >= 0.3 medium, else low. A 1.2× factor is enough
to move anything in [0.583, 0.7) or [0.25, 0.3) across a boundary, and it moves it the wrong
way: the files that are actually untested get inflated, the test files get deflated, and a
well-tested hot module reads as riskier than an untested one.

Measured vs inferred

Measured: the whole table, both surfaces, on the published 3.1.4 image, and the ratio.
Read, not run: blast_radius.rs:237 and compute_blast_impact's doc comment, from main at
3a5f384 — the same code the published image was built from for this path.

One row is supporting rather than decisive and is labelled as such: dep4.py has
tests/test_dep4.py present, which is exactly the candidate find_test_files_by_name generates
for it (tests/test_{stem}.py), and it scores identically to dep1.py — a file with nothing.
That is consistent with declared coverage being ignored, but it rests on that one source reading
of the candidate list, since #84 means a name-linked test cannot be observed in the output.

Provenance: unknown — not investigated; my checkout is a --depth 1 clone. Last known good: none.

Repro: ~/qa-scratch/mkrisk.py (builds the fixture), ~/qa-scratch/riskprobe.py.

Sibling: #84 — same function, the other consequence of the same test map being consulted in a way
that cannot see half of it.

Acceptance criteria

  • Two structurally identical dependents, one under tests/ and one not, neither with any test
    coverage, receive the same risk. That assertion fails against today's code.
  • A source file with a test in the map scores lower than an identical file without one — the
    direction compute_blast_impact's doc states.
  • If scoring test files as lower-risk is wanted for its own sake, it should be a separate
    adjustment with its own name, not the has_test_coverage argument.

Metadata

Metadata

Assignees

No one assigned

    Labels

    class-CBroken promise: runs, but violates a CONTRACT.md guarantee

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions