fix(claude_code): bugbot — aggregate all fail errors + structural test every manifest feature

Addresses three Bugbot concerns flagged on PR #28027 that are real
behavioral / coverage gaps:

1. matrix_builder._aggregate_cell now joins every failing tier's error
   in the published cell instead of silently dropping all but the first.
   When Haiku 429s and Opus times out on the same cell, both diagnostics
   land in the matrix JSON so docs-page triage can name both outliers.

2. _aggregate_cell treats 'not_tested' rows as absent data: they're
   dropped before computing the cell status. Previously a mixed
   (pass, not_tested) cell silently fell through to 'not_tested',
   discarding the passing tiers and hiding real coverage from the
   published matrix. A cell still aggregates to 'not_tested' when
   *every* row is 'not_tested' (or there are no rows at all).

3. test_v0_layout.py now structurally validates every feature declared
   in manifest.yaml (directory exists, __init__.py exists, every
   per-provider test_<provider>.py exists), not just the original six
   v0 rows. The EXPECTED_FEATURE_IDS / EXPECTED_PROVIDERS anchor
   constants still pin v0 positions; the new manifest-driven tests
   extend the same structural guarantees to every post-v0 row so a
   broken directory in 'count_tokens', 'tool_search', 'web_search',
   etc. fails CI instead of silently becoming a 'not_tested' cell.

Three new builder tests pin the new aggregation behavior:
  - mixed pass + not_tested surfaces as pass
  - all-not_tested stays not_tested
  - multiple fail errors are joined with '; '

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
This commit is contained in:
Cursor Agent 2026-05-18 03:48:23 +00:00
parent 83ea86718b
commit 9928da27f3
No known key found for this signature in database
3 changed files with 167 additions and 6 deletions

View file

@ -90,6 +90,105 @@ def test_build_matrix_any_fail_makes_cell_fail():
assert cell["error"] == "[claude-opus-4-7] timeout"
def test_build_matrix_joins_all_failure_errors_in_one_cell():
"""When multiple tiers fail for different reasons within the same cell,
every failure's error must appear in the published cell so triage
isn't reduced to a single tier's diagnostic.
"""
manifest = {
"schema_version": "1",
"providers": ["anthropic"],
"features": [{"id": "f", "name": "F"}],
}
results = [
{
"feature_id": "f",
"provider": "anthropic",
"result": {"status": "fail", "error": "[claude-haiku-4-5] 429"},
},
{"feature_id": "f", "provider": "anthropic", "result": {"status": "pass"}},
{
"feature_id": "f",
"provider": "anthropic",
"result": {"status": "fail", "error": "[claude-opus-4-7] timeout"},
},
]
matrix = build_matrix(
manifest=manifest,
results=results,
litellm_version="v",
claude_code_version="c",
generated_at="t",
)
cell = matrix["features"][0]["providers"]["anthropic"]
assert cell["status"] == "fail"
assert "[claude-haiku-4-5] 429" in cell["error"]
assert "[claude-opus-4-7] timeout" in cell["error"]
def test_build_matrix_mixed_pass_and_not_tested_surfaces_pass():
"""A `not_tested` row mixed with `pass` rows must not silently demote
the cell to `not_tested` `not_tested` is "absent data", not a
negative signal. Otherwise a partial crash mid-test, or a test that
explicitly recorded "tier didn't run", would discard real passing
results from the published cell.
"""
manifest = {
"schema_version": "1",
"providers": ["anthropic"],
"features": [{"id": "f", "name": "F"}],
}
results = [
{"feature_id": "f", "provider": "anthropic", "result": {"status": "pass"}},
{
"feature_id": "f",
"provider": "anthropic",
"result": {"status": "not_tested"},
},
{"feature_id": "f", "provider": "anthropic", "result": {"status": "pass"}},
]
matrix = build_matrix(
manifest=manifest,
results=results,
litellm_version="v",
claude_code_version="c",
generated_at="t",
)
assert matrix["features"][0]["providers"]["anthropic"] == {"status": "pass"}
def test_build_matrix_all_not_tested_stays_not_tested():
"""A cell whose every row is `not_tested` (or empty) must remain
`not_tested` the absent-data rule only drops `not_tested` rows
when there's other signal to surface.
"""
manifest = {
"schema_version": "1",
"providers": ["anthropic"],
"features": [{"id": "f", "name": "F"}],
}
results = [
{
"feature_id": "f",
"provider": "anthropic",
"result": {"status": "not_tested"},
},
{
"feature_id": "f",
"provider": "anthropic",
"result": {"status": "not_tested"},
},
]
matrix = build_matrix(
manifest=manifest,
results=results,
litellm_version="v",
claude_code_version="c",
generated_at="t",
)
assert matrix["features"][0]["providers"]["anthropic"] == {"status": "not_tested"}
def test_build_matrix_fills_not_tested_for_missing_cells():
manifest = {
"schema_version": "1",

View file

@ -46,6 +46,23 @@ EXPECTED_PROVIDERS = [
]
def _all_manifest_feature_ids() -> list[str]:
"""Every feature_id currently declared in `manifest.yaml`.
Evaluated at import time so the result can drive parametrized
structural tests below. Used to catch layout drift on post-v0
feature rows added after the matrix shipped the v0 anchor
constants above only validate the original six rows by design.
"""
return [
feature["id"]
for feature in yaml.safe_load(MANIFEST_PATH.read_text())["features"]
]
ALL_FEATURE_IDS = _all_manifest_feature_ids()
@pytest.fixture(scope="module")
def manifest() -> dict:
return yaml.safe_load(MANIFEST_PATH.read_text())
@ -93,6 +110,37 @@ def test_feature_directory_has_init_file(feature_id):
assert init_file.is_file(), f"missing __init__.py: {init_file}"
# Manifest-driven structural tests: every feature in `manifest.yaml`
# (v0 and post-v0 alike) must have the expected on-disk layout. The
# v0-only tests above pin the position of the original six rows; these
# extend the same structural guarantees to any row added afterward so
# a broken post-v0 directory still fails CI.
@pytest.mark.parametrize("feature_id", ALL_FEATURE_IDS)
def test_every_manifest_feature_has_directory(feature_id):
feature_dir = REPO_ROOT / feature_id
assert feature_dir.is_dir(), (
f"manifest declares {feature_id!r} but {feature_dir} is missing — "
"feature_id MUST match its on-disk directory (see manifest.yaml header)."
)
@pytest.mark.parametrize("feature_id", ALL_FEATURE_IDS)
def test_every_manifest_feature_has_init_file(feature_id):
init_file = REPO_ROOT / feature_id / "__init__.py"
assert init_file.is_file(), f"missing __init__.py: {init_file}"
@pytest.mark.parametrize("feature_id", ALL_FEATURE_IDS)
@pytest.mark.parametrize("provider", EXPECTED_PROVIDERS)
def test_every_manifest_feature_has_per_provider_test_file(feature_id, provider):
"""Every (feature, provider) cell in the rendered matrix must be
backed by a per-provider test file. Without this check, a missing
file silently becomes a `not_tested` cell in the published matrix
rather than a CI failure surfacing the layout drift."""
test_file = REPO_ROOT / feature_id / f"test_{provider}.py"
assert test_file.is_file(), f"missing per-provider test file: {test_file}"
@pytest.mark.parametrize("feature_id", EXPECTED_FEATURE_IDS)
@pytest.mark.parametrize("provider", EXPECTED_PROVIDERS)
def test_per_provider_test_file_imports_and_parametrizes_three_models(

View file

@ -130,26 +130,40 @@ def _aggregate_cell(results: Sequence[Mapping[str, Any]]) -> Dict[str, Any]:
"""Aggregate a list of per-model results into a single cell status.
Order of precedence (most informative wins):
- Any `fail` cell is `fail` with the first failure's error.
- Any `fail` cell is `fail` with every failing model's error
joined by `"; "` so a multi-tier breakage doesn't silently hide
all but the first error from the published matrix.
- `not_applicable` cell is `not_applicable` with the reason.
- `pass` cell is `pass`.
- empty / nothing recognized `not_tested`.
`not_tested` rows are treated as absent data: they're dropped before
aggregation so a mix of (pass, not_tested) e.g. from a partial
crash or a test that explicitly recorded "this tier didn't run"
still surfaces the passing tiers rather than silently demoting the
whole cell to `not_tested`. A cell is only `not_tested` when *every*
row is `not_tested` (or there are no rows at all).
"""
if not results:
return {"status": "not_tested"}
for r in results:
if r.get("status") == "fail":
return {"status": "fail", "error": str(r.get("error", "test failed"))}
observed = [r for r in results if r.get("status") != "not_tested"]
if not observed:
return {"status": "not_tested"}
for r in results:
failures = [r for r in observed if r.get("status") == "fail"]
if failures:
errors = [str(r.get("error", "test failed")) for r in failures]
return {"status": "fail", "error": "; ".join(errors)}
for r in observed:
if r.get("status") == "not_applicable":
return {
"status": "not_applicable",
"reason": str(r.get("reason", "not applicable")),
}
if all(r.get("status") == "pass" for r in results):
if all(r.get("status") == "pass" for r in observed):
return {"status": "pass"}
return {"status": "not_tested"}