diff --git a/docs/layers/CODE_HEALTH.md b/docs/layers/CODE_HEALTH.md index 4c4b16a277..12641c2c0b 100644 --- a/docs/layers/CODE_HEALTH.md +++ b/docs/layers/CODE_HEALTH.md @@ -28,14 +28,20 @@ The dashboard (`repowise serve`) has a Code Health page per repository at ``` Fix first (3 of 41 eligible items, 12 due; 230 in the inventory, tests, tooling and history-only files left out) - 1. Extract the retry branch from parse_config now, effort M, +1.4 health + 1. Extract the retry branch from parse_config now · effort M · +1.4 health on this file src/config/loader.py:212 CCN 47 and nesting 6 in a file changed 19 times this quarter. verify: pytest tests/unit/test_loader.py -Code health: 7.4/10 [Good], Hotspot: 6.1/10, Worst: 2.3/10 (src/config/loader.py) +Code health: 7.42/10 [Good] · Production code: 6.93/10 [Fair] · Hotspot: 6.10/10 [Fair] · Worst: 2.3/10 [At risk] (src/config/loader.py) ``` +Every score on the headline carries its band; Production code leaves test files out and +shows while tests are in the reading. In `repowise next`, a row's severity is the worst of +the findings it rests on, held at high. A coverage figure measured at an earlier commit +than the index says so on its row, and the refresh row appears once that report is 14 days +older than the latest commit. + ## The three signals Three signals come from one stream of findings, each with its own weights and caps, and diff --git a/packages/cli/src/repowise/cli/commands/health_cmd/command.py b/packages/cli/src/repowise/cli/commands/health_cmd/command.py index 0e2ef44e3e..1b4b3c490d 100644 --- a/packages/cli/src/repowise/cli/commands/health_cmd/command.py +++ b/packages/cli/src/repowise/cli/commands/health_cmd/command.py @@ -604,33 +604,40 @@ def _render_headline(kpis: dict[str, Any], code_shape: bool, unanalysed: int) -> BAND_LABEL, BAND_TERMINAL_COLOR, analysed_note, + band_for, headline_band, ) + share = kpis.get("analysed_share") + + def banded(label: str, score: Any, band: str | None) -> str: + # Every score printed carries its band, so a Fair figure never sits + # unlabelled beside a Good one. + tag = "" + if band is not None: + color = BAND_TERMINAL_COLOR[band] + tag = f" [[{color}]{BAND_LABEL[band]}[/{color}]]" + return f"{label}: [bold]{score if score is not None else '?'}[/bold]/10{tag}" + avg = kpis.get("average_health") - band_str = "" - band = headline_band(avg, kpis.get("analysed_share")) - if band is not None: - band_color = BAND_TERMINAL_COLOR[band] - band_str = f" [[{band_color}]{BAND_LABEL[band]}[/{band_color}]]" + avg_band = headline_band(avg, share) + parts = [banded("Code health", avg, avg_band)] # Production code beside the all-files figure, while tests are in it. production = kpis.get("production_average") has_tests = (kpis.get("production_file_count") or 0) < (kpis.get("file_count") or 0) - production_str = ( - f"Production code: [bold]{production}[/bold]/10 · " - if production is not None and has_tests - else "" - ) + if production is not None and has_tests: + parts.append(banded("Production code", production, headline_band(production, share))) # A repo with no recent churn has no hotspots, so no hotspot figure. hotspot = kpis.get("hotspot_health") - hotspot_str = f"Hotspot: [bold]{hotspot}[/bold]/10 · " if hotspot is not None else "" - console.print( - f"\nCode health: [bold]{avg if avg is not None else '?'}[/bold]/10{band_str} · " - f"{production_str}" - f"{hotspot_str}" - f"Worst: [bold]{kpis.get('worst_performer_score') or '?'}[/bold]/10 " - f"({kpis.get('worst_performer_path') or 'n/a'})" + if hotspot is not None: + parts.append(banded("Hotspot", hotspot, headline_band(hotspot, share))) + # One file's score takes the file's band, withheld with the headline's. + worst = kpis.get("worst_performer_score") + parts.append( + banded("Worst", worst, band_for(worst) if worst is not None and avg_band else None) + + f" ({kpis.get('worst_performer_path') or 'n/a'})" ) + console.print("\n" + " · ".join(parts)) if code_shape: console.print("[dim]Counting code shape only. Change history is left out.[/dim]") if unanalysed: diff --git a/packages/core/src/repowise/core/analysis/actions/model.py b/packages/core/src/repowise/core/analysis/actions/model.py index a861250302..cb9d2633ef 100644 --- a/packages/core/src/repowise/core/analysis/actions/model.py +++ b/packages/core/src/repowise/core/analysis/actions/model.py @@ -7,6 +7,7 @@ from __future__ import annotations import hashlib +from collections.abc import Iterable from dataclasses import dataclass from typing import Any, Literal, get_args @@ -88,6 +89,17 @@ SEVERITY_VALUE: dict[str, int] = {"critical": 4, "high": 3, "medium": 2, "low": 1} +def evidence_severity(severities: Iterable[str | None], default: str) -> str: + """An action's severity from the findings it rests on: the worst of them, + held at high (critical is a live credential's, see :attr:`Action.severity`). + ``default`` when it rests on none, so a row and its evidence never disagree.""" + known = [s for s in severities if s in SEVERITY_VALUE] + if not known: + return default + worst = max(known, key=SEVERITY_VALUE.__getitem__) + return "high" if worst == "critical" else worst + + def effective_value(value: int | None, severity: str) -> int: """An action's value: its own, else its severity's (also for a view stored before actions carried one).""" diff --git a/packages/core/src/repowise/core/analysis/actions/rules/code.py b/packages/core/src/repowise/core/analysis/actions/rules/code.py index ae776cfa40..168d43ecb7 100644 --- a/packages/core/src/repowise/core/analysis/actions/rules/code.py +++ b/packages/core/src/repowise/core/analysis/actions/rules/code.py @@ -16,6 +16,7 @@ ActionDetail, RuleOutcome, WhyFact, + evidence_severity, fingerprint, shared_priority, ) @@ -135,11 +136,12 @@ def _regression_file(path: str, found: list, ctx: RepoContext, weight: float) -> else: title = f"Fix what this week's changes added to {code(path)}" latest = max(commits.values(), key=lambda f: f.committed_at or ctx.anchor) + details = _regression_details(found) return Action( rule="fresh_regressions", tier="act_now", horizons=("week",), - severity="high", + severity=_details_severity(details, "high"), title=title, impact=( f"{plural(len(commits), 'commit')} in the last 7 days added " @@ -164,7 +166,7 @@ def _regression_file(path: str, found: list, ctx: RepoContext, weight: float) -> evidence_ids=tuple(sorted(commits)), evidence_total=len(found), fingerprint=fingerprint(len(found), *sorted(kinds)), - details=_regression_details(found), + details=details, commands=_regression_commands([path], latest.sha), ) @@ -188,6 +190,10 @@ def _regression_details(found: list) -> tuple[ActionDetail, ...]: ) +def _details_severity(details: tuple[ActionDetail, ...], default: str) -> str: + return evidence_severity((d.severity for d in details), default) + + def _regression_commands(paths: list[str], sha: str) -> tuple[ActionCommand, ...]: first = paths[0] return ( @@ -216,11 +222,15 @@ def _regression_rollup(ranked: list[tuple[str, list]]) -> Action: found = [f for _, fs in ranked for f in fs] commits = {f.sha for f in found} worst_path, worst = ranked[0] + # Evidence in the order the files are ranked, so it starts where the + # impact line says to start; the commit to inspect is the latest one + # that touched that same file. + details = tuple(d for _, fs in ranked for d in _regression_details(fs)) return Action( rule="fresh_regressions", tier="act_now", horizons=("week",), - severity="high", + severity=_details_severity(details, "high"), title=( f"Fix what this week's commits added: {_kinds_text(found)} " f"in {len(ranked):,} files" @@ -246,10 +256,7 @@ def _regression_rollup(ranked: list[tuple[str, list]]) -> Action: evidence_total=len(found), includes=tuple(path for path, _ in ranked[:10]), fingerprint=fingerprint(len(found) // 10, *sorted({f.kind for f in found})), - # Evidence in the order the files are ranked, so it starts where the - # impact line says to start; the commit to inspect is the latest one - # that touched that same file. - details=tuple(d for _, fs in ranked for d in _regression_details(fs)), + details=details, details_total=len(found), commands=_regression_commands( [path for path, _ in ranked], @@ -268,11 +275,12 @@ def _shape_rollup(ranked: list[tuple[str, list]]) -> Action: commits = {f.sha for f in found} worst_path, worst = ranked[0] where = code(worst_path) if len(ranked) == 1 else f"{len(ranked):,} files" + details = tuple(d for _, fs in ranked for d in _regression_details(fs)) return Action( rule="fresh_regressions", tier="plan", horizons=("week",), - severity="medium", + severity=_details_severity(details, "medium"), title=( f"Tidy the code shape this week's commits left: " f"{plural(len(found), 'finding')} in {where}" @@ -303,7 +311,7 @@ def _shape_rollup(ranked: list[tuple[str, list]]) -> Action: evidence_total=len(found), includes=tuple(path for path, _ in ranked[:10]), fingerprint=fingerprint("shape", len(found) // 10), - details=tuple(d for _, fs in ranked for d in _regression_details(fs)), + details=details, details_total=len(found), commands=_regression_commands( [path for path, _ in ranked], @@ -317,7 +325,7 @@ def _coverage_fact(f: FileFacts, facts: RepoFacts) -> WhyFact: basis = "measured" value = f"{f.line_coverage_pct:.0f}%" if facts.coverage.status == "stale": - value += " (older report)" + value += " (measured at an earlier commit)" return WhyFact("line coverage", value, basis) if facts.coverage.status == "unknown": return WhyFact("line coverage", "Unknown, no report", "unknown") @@ -689,8 +697,13 @@ def fix_first(facts: RepoFacts, ctx: RepoContext) -> RuleOutcome: continue effort = _effort(item.effort.bucket) confidence = "high" if item.confidence.level == "high" else "medium" - severity = "high" if tier == "act_now" else "medium" - ceiling = min(ceiling, shared_priority(item.value, severity, confidence, effort)) + tier_severity = "high" if tier == "act_now" else "medium" + # A finding item's severity fact is its evidence; a performance or + # refactoring item carries none and reads its tier. Order stays the tier's. + severity = evidence_severity( + (f.value for f in item.facts if f.label == "severity"), tier_severity + ) + ceiling = min(ceiling, shared_priority(item.value, tier_severity, confidence, effort)) target = item.target symbol = target.symbol actions.append( diff --git a/packages/core/src/repowise/core/analysis/actions/rules/signal.py b/packages/core/src/repowise/core/analysis/actions/rules/signal.py index 909ba8b119..a6acba6872 100644 --- a/packages/core/src/repowise/core/analysis/actions/rules/signal.py +++ b/packages/core/src/repowise/core/analysis/actions/rules/signal.py @@ -15,6 +15,8 @@ #: Below this many production files, a coverage report is not worth asking for. COVERAGE_MIN_FILES = 20 +#: Coverage is stale once measured at an earlier commit than the index's +#: (``build.build_coverage``); a refresh is asked for only once it is this old. COVERAGE_STALE_AFTER = timedelta(days=14) DECISIONS_REVIEW_MIN = 5 @@ -57,7 +59,12 @@ def coverage_stale(facts: RepoFacts, ctx: RepoContext) -> RuleOutcome: return RuleOutcome(rule, "evaluated", "Coverage is current or absent.") age = ctx.anchor.replace(tzinfo=None) - cov.ingested_at.replace(tzinfo=None) if age < COVERAGE_STALE_AFTER: - return RuleOutcome(rule, "evaluated", "Coverage is recent enough.") + return RuleOutcome( + rule, + "evaluated", + "Coverage was measured at an earlier commit; a refresh is asked for once " + f"the report is {COVERAGE_STALE_AFTER.days} days older than the latest commit.", + ) action = Action( rule=rule, tier="improve_signal", diff --git a/tests/unit/analysis/test_actions_rules.py b/tests/unit/analysis/test_actions_rules.py index 2fc4d56022..3f1b90518f 100644 --- a/tests/unit/analysis/test_actions_rules.py +++ b/tests/unit/analysis/test_actions_rules.py @@ -205,7 +205,8 @@ def test_code_shape_from_the_week_is_one_maintainability_item_to_plan() -> None: ) for subset in (found, found[:1]): (a,) = _run(code.fresh_regressions, _facts(recent_findings=subset)).actions - assert (a.tier, a.severity, a.effective_value) == ("plan", "medium", SEVERITY_VALUE["medium"]) + # Its severity is its evidence's, held at high; its value keeps it below defects. + assert (a.tier, a.severity, a.effective_value) == ("plan", "high", SEVERITY_VALUE["medium"]) assert a.horizons == ("week",) assert dict((w.label, w.value) for w in a.why)["code-shape findings"] == str(len(subset)) (single,) = _run(code.fresh_regressions, _facts(recent_findings=found[:1])).actions @@ -394,6 +395,34 @@ def test_fix_first_emits_now_and_next_and_leaves_later() -> None: assert len({a.action_id for a in actions}) == 2 +def test_a_fix_first_row_reads_its_findings_severity() -> None: + """GraphFlow's row said medium while its own severity fact said high.""" + from repowise.core.analysis.health.fix_first.model import FixFact + + item = _fix_items()[0] + for fact, tier, expected in ( + ("high", "next", "high"), + ("critical", "now", "high"), + ("medium", "now", "medium"), + (None, "next", "medium"), + (None, "now", "high"), + ): + facts = (FixFact("severity", fact),) if fact else () + (a,) = _run(code.fix_first, _facts(fix_first=(replace(item, tier=tier, facts=facts),))).actions + assert a.severity == expected, (fact, tier) + if fact: + assert dict((w.label, w.value) for w in a.why)["severity"] == fact + + +def test_a_regression_row_reads_its_findings_severity() -> None: + def severity(*found: RecentFinding) -> str: + return _run(code.fresh_regressions, _facts(recent_findings=found)).actions[0].severity + + assert severity(_recent("src/a.py", severity="medium")) == "medium" + assert severity(_recent("src/a.py", severity="critical", kind="performance")) == "high" + assert severity(_recent("src/a.py", severity="low", kind="shape")) == "low" + + def test_fix_first_leaves_a_cause_the_week_added_to_fresh_regressions() -> None: perf = _fix_items()[0] assert perf.kind == "perf_fix" and perf.target.symbol @@ -563,7 +592,15 @@ def test_coverage_stale_is_improve_signal() -> None: (a,) = _run(signal.coverage_stale, _facts(coverage=cov)).actions assert a.tier == "improve_signal" recent = CoverageState("stale", ANCHOR - timedelta(days=3), 40) - assert _run(signal.coverage_stale, _facts(coverage=recent)).actions == () + outcome = _run(signal.coverage_stale, _facts(coverage=recent)) + assert outcome.actions == () + # Stale is one thing everywhere: measured at an earlier commit. + assert outcome.reason.startswith("Coverage was measured at an earlier commit;") + file = _file("src/a.py", line_coverage_pct=62.0) + (row,) = _run(code.fragile_file, _facts([file], coverage=recent)).actions + assert dict((w.label, w.value) for w in row.why)["line coverage"] == ( + "62% (measured at an earlier commit)" + ) def test_decisions_unreviewed_is_improve_signal() -> None: diff --git a/tests/unit/cli/test_health_cmd_controls.py b/tests/unit/cli/test_health_cmd_controls.py index cc316f09ae..68a71fcf71 100644 --- a/tests/unit/cli/test_health_cmd_controls.py +++ b/tests/unit/cli/test_health_cmd_controls.py @@ -180,6 +180,7 @@ def test_a_mostly_unanalysed_repo_gets_no_band_in_any_scope() -> None: for args in ((), ("--scope", "production")): out = _mostly_unanalysed_output(*args) assert "Code health: 10.0/10 ·" in out, args + # No band anywhere on the line, the worst file's included. assert "Excellent" not in out, args assert "analysed 4 of 435 code files" in out, args @@ -192,7 +193,10 @@ def headline(**kpis) -> str: return console.file.getvalue() base = {"average_health": 8.0, "production_average": 6.0, "file_count": 2} - assert "Production code: 6.0/10 · Worst:" in headline(**base, production_file_count=1) + # Every score carries its band: 6.0 is Fair beside a Good headline. + assert "[Good] · Production code: 6.0/10 [Fair] · Worst:" in headline( + **base, production_file_count=1 + ) # Narrowed to production, the two figures are one. narrowed = headline(**base, production_file_count=2) assert "Production code" not in narrowed and "[Good] · Worst:" in narrowed @@ -205,7 +209,8 @@ def headline(**kpis) -> str: health_module._render_headline(kpis, False, 0) return console.file.getvalue() - assert "Hotspot: 6.5/10 · Worst:" in headline(average_health=8.0, hotspot_health=6.5) + out = headline(average_health=8.0, hotspot_health=6.5, worst_performer_score=1.0) + assert "Hotspot: 6.5/10 [Fair] · Worst: 1.0/10 [At risk]" in out assert "Hotspot" not in headline(average_health=8.0, hotspot_health=None) diff --git a/tests/unit/persistence/test_actions_loader.py b/tests/unit/persistence/test_actions_loader.py index 2d6d1aa7e2..d5814956fb 100644 --- a/tests/unit/persistence/test_actions_loader.py +++ b/tests/unit/persistence/test_actions_loader.py @@ -223,8 +223,9 @@ async def test_load_actions_view_composes_every_store(async_session) -> None: perf, shape = sorted(week["fresh_regressions"], key=lambda a: a["tier"]) assert (perf["tier"], perf["target"]["path"]) == ("act_now", "src/repo.py") assert perf["title"] == "Fix the performance regression in `load` in `src/repo.py`" + # Planned, at its finding's severity. assert (shape["tier"], shape["severity"], shape["target"]["path"]) == ( - "plan", "medium", "src/new.py" + "plan", "high", "src/new.py" ) assert shape["evidence_ids"] == ["recent"]