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 d2cdd93d0..57b9d7a4b 100644 --- a/packages/cli/src/repowise/cli/commands/health_cmd/command.py +++ b/packages/cli/src/repowise/cli/commands/health_cmd/command.py @@ -23,6 +23,8 @@ run_async, silence_logs_for_machine_output, ) +from repowise.core.analysis.health.scope import DEFAULT_SCOPE, SCOPES, parse_scope +from repowise.core.analysis.health.scoring import compute_kpis from .codegen import _generate_refactoring_code from .persist import _load_persisted_coverage_map, _load_recommendations, _persist_health @@ -32,6 +34,7 @@ _render_defect_accuracy_line, _render_distribution_line, _render_performance_section, + _render_split_line, ) from .trends import _render_trend @@ -87,6 +90,15 @@ default=None, help="Restrict the report to files whose path starts with this prefix.", ) +@click.option( + "--scope", + default=DEFAULT_SCOPE, + type=click.Choice(list(SCOPES)), + help=( + "Which files to report on. Tests score higher than production code, " + "so 'production' lowers every figure without a defect being found." + ), +) @click.option( "--trend", "trend_view", @@ -117,6 +129,7 @@ def health_command( refactoring_targets: bool, generate_code: str | None, module_filter: str | None, + scope: str, trend_view: bool, badge_view: bool, verbose: bool, @@ -266,13 +279,24 @@ def health_command( metrics = [m for m in metrics if m.file_path == file_filter] if module_filter: metrics = [m for m in metrics if m.file_path.startswith(module_filter)] + if parse_scope(scope) == "production": + metrics = [m for m in metrics if not m.is_test] + # Every figure, not just the table: the flag says production, so the + # headline, the hotspot number, the worst performer and the + # distribution all have to describe that population too. + report.kpis = compute_kpis( + metrics, {p for p, m in git_meta_map.items() if m.get("is_hotspot")} + ) metrics_sorted = sorted(metrics, key=lambda m: m.score) + scoped_paths = {m.file_path for m in metrics} findings = report.findings if file_filter: findings = [f for f in findings if f.file_path == file_filter] if module_filter: findings = [f for f in findings if f.file_path.startswith(module_filter)] + if parse_scope(scope) == "production": + findings = [f for f in findings if f.file_path in scoped_paths] if generate_code is not None: suggestions = getattr(report, "refactoring_suggestions", None) or [] @@ -366,12 +390,13 @@ def health_command( band_color = {"healthy": "green", "warning": "yellow", "alert": "red"}[band] band_str = f" [[{band_color}]{BAND_LABEL[band]}[/{band_color}]]" console.print( - f"\nHotspot: [bold]{kpis.get('hotspot_health', '?')}[/bold]/10 · " - f"Average: [bold]{avg if avg is not None else '?'}[/bold]/10{band_str} · " + f"\nCode health: [bold]{avg if avg is not None else '?'}[/bold]/10{band_str} · " + f"Hotspot: [bold]{kpis.get('hotspot_health', '?')}[/bold]/10 · " f"Worst: [bold]{kpis.get('worst_performer_score', '?')}[/bold]/10 " f"({kpis.get('worst_performer_path', 'n/a')})" ) - _render_distribution_line(health_distribution(report.metrics)) + _render_split_line(kpis) + _render_distribution_line(health_distribution(metrics)) _render_defect_accuracy_line(report) diff --git a/packages/cli/src/repowise/cli/commands/health_cmd/summary.py b/packages/cli/src/repowise/cli/commands/health_cmd/summary.py index 516197ed3..a333303b4 100644 --- a/packages/cli/src/repowise/cli/commands/health_cmd/summary.py +++ b/packages/cli/src/repowise/cli/commands/health_cmd/summary.py @@ -54,6 +54,18 @@ def _render_performance_section(report: Any, lang_by_path: dict[str, str]) -> No ) +def _render_split_line(kpis: dict) -> None: + """The headline's two halves, so a reader can see which one holds it down.""" + structure = kpis.get("structure_average") + history = kpis.get("history_average") + if structure is None or history is None: + return + console.print( + f"[dim]Of that deduction, [/dim]{structure:.2f}[dim] is code shape and [/dim]" + f"{history:.2f}[dim] is history — history answers to time, not to editing.[/dim]" + ) + + def _render_distribution_line(dist: dict) -> None: """One compact line: the NLOC-weighted file split across the 3 bands.""" bands = dist.get("bands") or {} diff --git a/packages/core/src/repowise/core/analysis/health/trends.py b/packages/core/src/repowise/core/analysis/health/trends.py index 912453599..eba98a990 100644 --- a/packages/core/src/repowise/core/analysis/health/trends.py +++ b/packages/core/src/repowise/core/analysis/health/trends.py @@ -50,6 +50,16 @@ class TrendAlert: baseline: float | None delta: float message: str + # Which half of the headline moved, in score points. ``structure`` is code + # shape, which a rewrite can fix; ``history`` is git-derived and answers to + # time, not to editing. They are changes in each half's mean DEDUCTION, so + # they sum to ``delta`` only while no file sits at the score floor — the + # clamp is what the two measures disagree about. All ``None`` for + # ``hotspot_health``, whose halves are not snapshotted, and on snapshots + # taken before the split existed. + driver: str | None = None + structure_delta: float | None = None + history_delta: float | None = None @dataclass @@ -63,6 +73,11 @@ class TrendSummary: hotspot_delta: float | None average_delta: float | None alerts: list[TrendAlert] = field(default_factory=list) + # The newest snapshot's headline split, in deduction points, so a surface + # can show the two halves without replaying findings. ``None`` before the + # split was recorded. + current_structure_deduction: float | None = None + current_history_deduction: float | None = None def _delta(current: float, previous: float | None) -> float | None: @@ -71,6 +86,92 @@ def _delta(current: float, previous: float | None) -> float | None: return round(current - previous, 3) +def _attribution(current: Any, baseline: Any) -> tuple[str | None, float | None, float | None]: + """Split a headline move into its structure and history halves. + + Snapshots store the two as deduction points, so each contribution to the + score is the negated change. Returns ``(driver, structure, history)``, all + ``None`` when either snapshot predates the split. + """ + values = [ + (getattr(snap, attr, None)) + for snap in (current, baseline) + for attr in ("structure_average", "history_average") + ] + if any(v is None for v in values): + return None, None, None + cur_structure, cur_history, base_structure, base_history = (float(v) for v in values) + structure = round(base_structure - cur_structure, 3) + history = round(base_history - cur_history, 3) + driver = "structure" if abs(structure) >= abs(history) else "history" + return driver, structure, history + + +_DRIVER_PHRASE = { + "structure": "Code shape moved most", + "history": "History moved most, and no edit to these files settles it", +} + + +def _driver_sentence(driver: str | None, structure: float | None, history: float | None) -> str: + """Name the driver and both halves, without claiming they sum to ``delta``. + + They are deduction means and ``delta`` is a mean of clamped scores, so the + two agree only on a repo with no floored file. Stating each half and + stopping there is the claim the numbers actually support. + """ + if driver is None or structure is None or history is None: + return "" + return f" {_DRIVER_PHRASE[driver]} (code shape {structure:+.2f}, history {history:+.2f})." + + +@dataclass +class _ScopedSnapshot: + """One snapshot with its production figure standing in for the headline. + + Only ``average_health`` was recorded for both populations. Everything else + on a snapshot describes the whole repository, and a repo-wide figure served + under a production label is worse than an absent one, so the rest is + dropped rather than carried across. + """ + + taken_at: Any + average_health: float + per_file_scores_json: str + hotspot_health: float = 0.0 + worst_performer_path: str | None = None + worst_performer_score: float | None = None + structure_average: float | None = None + history_average: float | None = None + + +def project_scope(history: list[Any], scope: str) -> list[Any]: + """Re-read a snapshot series through one scope. + + The default scope is what the rows already hold. Narrowing swaps in the + stored production average and drops snapshots taken before it was + recorded — a gap in the line is honest where a repo-wide number wearing a + production label would not be. + """ + from .scope import parse_scope + + if parse_scope(scope) != "production": + return history + out: list[Any] = [] + for snap in history: + value = getattr(snap, "production_average", None) + if value is None: + continue + out.append( + _ScopedSnapshot( + taken_at=snap.taken_at, + average_health=float(value), + per_file_scores_json=snap.per_file_scores_json, + ) + ) + return out + + def diff_snapshots(history: list[Any]) -> TrendSummary: """Compare the newest snapshot against the window behind it. @@ -103,6 +204,8 @@ def diff_snapshots(history: list[Any]) -> TrendSummary: float(current.average_health), float(prior.average_health) if prior else None, ), + current_structure_deduction=getattr(current, "structure_average", None), + current_history_deduction=getattr(current, "history_average", None), ) summary.alerts.extend(_declining_alerts(history)) @@ -140,6 +243,11 @@ def _declining_alerts(history: list[Any]) -> list[TrendAlert]: base_val = float(getattr(baseline, metric)) delta = round(cur_val - base_val, 3) if delta <= -DECLINE_THRESHOLD: + driver, structure, history = ( + _attribution(current, baseline) + if metric == "average_health" + else (None, None, None) + ) out.append( TrendAlert( kind="declining", @@ -152,7 +260,11 @@ def _declining_alerts(history: list[Any]) -> list[TrendAlert]: f"{abs(delta):.2f} points vs. snapshot " f"{DECLINE_LOOKBACK} ago " f"({base_val:.2f} → {cur_val:.2f})." + f"{_driver_sentence(driver, structure, history)}" ), + driver=driver, + structure_delta=structure, + history_delta=history, ) ) return out @@ -169,6 +281,11 @@ def _predicted_decline_alerts(history: list[Any]) -> list[TrendAlert]: vals = [float(getattr(s, metric)) for s in tail] if all(vals[i + 1] < vals[i] for i in range(len(vals) - 1)): delta = round(vals[-1] - vals[0], 3) + driver, structure, history = ( + _attribution(tail[-1], tail[0]) + if metric == "average_health" + else (None, None, None) + ) out.append( TrendAlert( kind="predicted_decline", @@ -180,12 +297,33 @@ def _predicted_decline_alerts(history: list[Any]) -> list[TrendAlert]: f"{metric.replace('_', ' ').title()} declined for " f"{PREDICTED_DECLINE_CONSECUTIVE} consecutive snapshots " f"({vals[0]:.2f} → {vals[-1]:.2f})." + f"{_driver_sentence(driver, structure, history)}" ), + driver=driver, + structure_delta=structure, + history_delta=history, ) ) return out +def drop_unscoped_fields(rows: list[dict[str, Any]]) -> list[dict[str, Any]]: + """Blank the per-row figures a narrowed snapshot never recorded. + + A projected snapshot carries the production average and nothing else, so + the placeholders the rest of the row picked up must not read as measurements. + """ + return [ + { + **row, + "hotspot_health": None, + "worst_performer_path": None, + "worst_performer_score": None, + } + for row in rows + ] + + def recent_kpis(history: list[Any], limit: int = 10) -> list[dict[str, Any]]: """Serialize the most-recent *limit* snapshots for CLI / API consumers. diff --git a/packages/server/src/repowise/server/mcp_server/tool_health.py b/packages/server/src/repowise/server/mcp_server/tool_health.py index 39653f7af..8c5e99b30 100644 --- a/packages/server/src/repowise/server/mcp_server/tool_health.py +++ b/packages/server/src/repowise/server/mcp_server/tool_health.py @@ -13,13 +13,12 @@ from repowise.core.analysis.health.aggregation import module_rollups as _module_rollups from repowise.core.analysis.health.churn_complexity import churn_complexity_points -from repowise.core.analysis.health.complexity.languages import LANGUAGE_MAPS from repowise.core.analysis.health.coverage import decay_since, measurement_ref from repowise.core.analysis.health.defect_accuracy import compute_defect_accuracy from repowise.core.analysis.health.finding_identity import finding_public_id from repowise.core.analysis.health.grading import HEALTHY_MIN, band_for from repowise.core.analysis.health.grading import distribution as health_distribution -from repowise.core.analysis.health.models import primary_finding +from repowise.core.analysis.health.models import primary_finding, split_by_origin from repowise.core.analysis.health.perf.coverage import PerfCoverage, coverage_for_metrics from repowise.core.analysis.health.perf.opportunity_rank import observation_rank from repowise.core.analysis.health.ranking import deduction_by_path, sort_metrics_worst_first @@ -28,11 +27,18 @@ build_recommendations, hydrate_recommendations, ) -from repowise.core.analysis.health.scoring import hotspot_health +from repowise.core.analysis.health.scope import DEFAULT_SCOPE, parse_scope +from repowise.core.analysis.health.scoring import hotspot_health, nloc_weighted_attr from repowise.core.analysis.health.semantics import health_semantics_contract from repowise.core.analysis.health.signals import file_signals from repowise.core.analysis.health.suggestions import suggestion_for -from repowise.core.analysis.health.trends import diff_snapshots, file_trend, recent_kpis +from repowise.core.analysis.health.trends import ( + diff_snapshots, + drop_unscoped_fields, + file_trend, + project_scope, + recent_kpis, +) from repowise.core.ingestion.models import FILE_DEPENDENCY_EDGE_TYPES from repowise.core.persistence.crud import ( get_all_git_metadata, @@ -707,9 +713,19 @@ def _leads_by_file(findings: list[Any]) -> dict[str, dict[str, Any]]: primary = primary_finding(fs) if primary is None: continue + # The same rule applied to code shape alone. A file can be led by a + # history marker, and a caller told to fix that has been handed + # something no edit resolves; ``watch_*`` carries it as context instead. + code_shape, history = split_by_origin(fs) + actionable = primary_finding(code_shape) + watch = primary_finding(history) leads[path] = { "primary_biomarker": primary.biomarker_type, "primary_reason": primary.reason, + "actionable_biomarker": actionable.biomarker_type if actionable else None, + "actionable_reason": actionable.reason if actionable else None, + "watch_biomarker": watch.biomarker_type if watch else None, + "watch_reason": watch.reason if watch else None, "total_deduction": round(sum(float(x.health_impact or 0.0) for x in fs), 3), } return leads @@ -902,7 +918,15 @@ def _directive( """ if not by_leverage: return None - top = by_leverage[0] + # The highest-leverage file that has something an edit can remove. A file + # whose whole deficit is history would otherwise be named ``fix_first`` + # with nothing to fix underneath it, which is the failure this block exists + # to prevent. Falls back to the top file when no candidate has one, and + # says so rather than inventing a task. + top = next( + (m for m in by_leverage if (leads.get(m.file_path) or {}).get("actionable_biomarker")), + by_leverage[0], + ) recovers = round(max(HEALTHY_MIN - top.score, 0.0) * max(top.nloc, 1)) lead = leads.get(top.file_path) or {} # Does anything behind ``plan_via`` actually address the cause named in @@ -910,12 +934,15 @@ def _directive( # biomarkers have no detector at all — ``coverage_gradient`` above all, which # no plan kind can answer because none of them writes tests. Saying so beats # routing the caller to plans for a different problem with full confidence. - lead_biomarker = lead.get("primary_biomarker") + # The cause named here has to be one an edit can remove, or the whole block + # recommends something impossible. History is reported alongside, in its own + # field, and never as ``reason``. + lead_biomarker = lead.get("actionable_biomarker") available = (plan_biomarkers_by_path or {}).get(top.file_path, set()) addresses = bool(lead_biomarker) and lead_biomarker in available out = { "fix_first": top.file_path, - "reason": lead.get("primary_reason") or f"scores {round(top.score, 2)}", + "reason": lead.get("actionable_reason") or f"scores {round(top.score, 2)}", # Points the repo headline recovers if this one file reaches Healthy, # and what share of the total gap that is — the "few files, not the # long tail" argument made concrete for a single file. The denominator @@ -930,7 +957,7 @@ def _directive( "equivalent_value": True, }, "share_of_repo_gap_pct": (round(100.0 * recovers / gap_points, 1) if gap_points else None), - "then": [m.file_path for m in by_leverage[1:3]], + "then": [m.file_path for m in by_leverage if m.file_path != top.file_path][:2], # Projected, not bare. ``include`` adds a block without subtracting the # dashboard, and five ranked lists at the default ``limit`` compose: the # bare ``include=['refactoring']`` measured 70,776 chars on this repo @@ -942,6 +969,22 @@ def _directive( "plan_via": "get_health(include=['refactoring'], only=['refactoring_plans'])", "plan_addresses_reason": addresses, } + # Context, not a task. These move with the repository's history and no edit + # to this file settles them. + if lead.get("watch_biomarker"): + out["watch"] = { + "biomarker": lead["watch_biomarker"], + "reason": lead.get("watch_reason"), + "note": "History-derived. Read it as context; there is nothing here to fix.", + } + if not lead_biomarker: + # Nothing in the repository has a code-shape lead, so the honest answer + # is that the deficit is history and no edit here settles it. + out["next_action"] = ( + "No file's leading cause is code shape; the deficit on this one is " + "history. Read watch for what is moving and leave it alone." + ) + return out # Only speak when there is a named cause to speak about. With no lead the # ``reason`` above already falls back to the bare score, and a note reading # "No stored plan addresses None" would be worse than silence. @@ -1216,22 +1259,6 @@ def _write_health_analysis_meta( meta["health_analysis"] = analysis -def _dimension_average(metrics: list[HealthFileMetric], attr: str) -> float | None: - """NLOC-weighted headline over a per-dimension score attribute. - - Skips rows without the attribute (those predating that pillar) so the KPI - reads "not measured" rather than a misleading 10.0; ``None`` when no row - carries it. - """ - scored = [m for m in metrics if getattr(m, attr, None) is not None] - if not scored: - return None - total_nloc = sum(max(m.nloc, 1) for m in scored) - if not total_nloc: - return round(sum(getattr(m, attr) for m in scored) / len(scored), 2) - return round(sum(getattr(m, attr) * max(m.nloc, 1) for m in scored) / total_nloc, 2) - - def _gap_analysis(metrics: list[HealthFileMetric]) -> dict[str, Any]: """How few files must reach 8.0 for the *weighted average* to reach 8.0. @@ -1330,17 +1357,10 @@ def _perf_kpis(performance_findings: int, coverage: PerfCoverage | None) -> dict } -def _code_only( - metrics: list[HealthFileMetric], lang_by_path: dict[str, str] -) -> list[HealthFileMetric]: - """The metric rows the complexity walker actually walks. - - ``LANGUAGE_MAPS`` is already the repo's definition of "real code" — the perf - pillar uses exactly this filter so docs/config rows never dilute its - coverage math (``perf/coverage.py::coverage_for_metrics``). The defect - headline never applied it. - """ - return [m for m in metrics if lang_by_path.get(m.file_path, "") in LANGUAGE_MAPS] +def _avg(metrics: list[HealthFileMetric], attr: str) -> float | None: + """NLOC-weighted mean of one metric column, rounded for the wire.""" + value = nloc_weighted_attr(metrics, attr) + return round(value, 2) if value is not None else None def _compute_kpis( @@ -1349,7 +1369,6 @@ def _compute_kpis( hotspot_paths: set[str] | None = None, performance_findings: int = 0, coverage: PerfCoverage | None = None, - lang_by_path: dict[str, str] | None = None, ) -> dict[str, Any]: if not metrics: return { @@ -1362,32 +1381,13 @@ def _compute_kpis( "worst_performer_score": None, "maintainability_average": None, "performance_average": None, + "structure_average": None, + "history_average": None, **_perf_kpis(0, None), } total_nloc = sum(max(m.nloc, 1) for m in metrics) avg = sum(m.score * max(m.nloc, 1) for m in metrics) / total_nloc worst = min(metrics, key=lambda r: r.score) - # What the headline would read over code alone. No biomarker walks a - # markdown or JSON file, so those rows carry a mechanical 10.0 that means - # "nothing looked at this", exactly the fabricated-10.0 problem the perf - # pillar already surfaces rather than hides (``perf/coverage.py``). Measured - # on this repo: 233 of 3,314 rows are non-code, 221 of them score exactly - # 10.0, they are 7.5% of NLOC, and they lift ``average_health`` 7.31 -> 7.47. - # Surfaced rather than subtracted: ``average_health`` is what the badge, the - # snapshots, the trend alerts and the web UI all read, and redefining it - # here alone would make this tool disagree with every one of them. - code_kpis: dict[str, Any] = {} - if lang_by_path is not None: - code = _code_only(metrics, lang_by_path) - code_nloc = sum(max(m.nloc, 1) for m in code) - code_kpis = { - "non_code_files": len(metrics) - len(code), - "average_health_code_only": ( - round(sum(m.score * max(m.nloc, 1) for m in code) / code_nloc, 2) - if code_nloc - else None - ), - } return { "file_count": len(metrics), "average_health": round(avg, 2), @@ -1396,7 +1396,6 @@ def _compute_kpis( # reporting it as a score, the same fabricated-10.0 problem the comment # above objects to for non-code rows. "hotspot_health": hotspot_health(metrics, hotspot_paths or set()), - **code_kpis, # NLOC-weighted (``average_health``) vs plain file mean. When these # diverge, a few large low-scoring files are holding the headline down — # the weighted number is what the dashboard/badge surface, and the gap @@ -1408,8 +1407,12 @@ def _compute_kpis( "worst_performer_score": round(worst.score, 2), # Maintainability + performance pillar headlines alongside the # defect-backed average. Each is ``None`` until its pillar is measured. - "maintainability_average": _dimension_average(metrics, "maintainability_score"), - "performance_average": _dimension_average(metrics, "performance_score"), + "maintainability_average": _avg(metrics, "maintainability_score"), + "performance_average": _avg(metrics, "performance_score"), + # The headline's two halves, in deduction points, so a caller can see + # whether code shape or history is holding the number down. + "structure_average": _avg(metrics, "structure_deduction"), + "history_average": _avg(metrics, "history_deduction"), # Performance leads with count + density + coverage, not the diluted /10. **_perf_kpis(performance_findings, coverage), } @@ -1494,6 +1497,7 @@ async def get_health( performance_boundary: str | None = None, performance_confidence: str | None = None, performance_sort: str | None = None, + scope: str = DEFAULT_SCOPE, ) -> dict: """Code-health scores and findings from stored analysis. @@ -1503,7 +1507,7 @@ async def get_health( Args: targets: file paths or ``module:``. Empty means dashboard; - unmatched ones land in ``unresolved``, surviving ``only``. + unmatched ones land in ``unresolved``. include: ``biomarkers`` | ``refactoring`` | ``trend`` | ``coverage`` | ``accuracy`` | ``signals`` | ``churn_complexity``, or a dimension. ``performance`` and ``refactoring`` add their queues. @@ -1514,15 +1518,15 @@ async def get_health( repo: usually omitted. limit: max rows per ranked list, ``0`` for none. cursor: zero-based offset into a ranked list. - finding_id: stable ``id`` from a health finding. - plan_id: stable ``id`` from a refactoring plan. - opportunity_id: ``perf...`` or ``refop...`` id from a directive or - queue: the unit, its steps or plan, and evidence paged by - ``only=["*_evidence"]``. Excludes the two ids above. + finding_id / plan_id: stable ``id`` from a finding or a plan. + opportunity_id: ``perf...`` or ``refop...`` id: the unit, its steps or + plan, and evidence paged by ``only=["*_evidence"]``. refactoring_view: ``diversified`` (default) | ``canonical`` | - ``file_spread``; refactoring_type / _confidence / _effort filter. + ``file_spread``; _type / _confidence / _effort filter. performance_view / _context / _boundary / _confidence / _sort: queue projection and filters; the facets list them. + scope: ``all`` (default) | ``production``. Narrowing drops test files + from every figure; tests score higher, so it lowers them. """ started = perf_counter() @@ -1764,8 +1768,25 @@ def bounded(rows: list[Any], label: str, *, cap: int | None = None) -> list[Any] # Paths the index knows about but the exclude config drops. Kept so an # unresolved target can report "excluded" (a config decision) rather # than "no_such_path" (a typo) — the two need different responses. + # Computed before ``scope`` narrows the list, or a test file would be + # reported as dropped by a config that says nothing about it. excluded_paths = {m.file_path for m in indexed_rows} - {m.file_path for m in all_metrics} + # Narrowing to production is the same shape of question as the exclude + # config: both drop whole files from every block at once. Folding it + # into one filter is what keeps a scoped dashboard from ranking a + # finding on a file its own file list no longer contains. + reported_scope = parse_scope(scope) + scope_paths: set[str] | None = None + if reported_scope == "production": + all_metrics = [m for m in all_metrics if not m.is_test] + scope_paths = {m.file_path for m in all_metrics} + + def in_scope_rows(rows: list, attr: str = "file_path") -> list: + rows = filter_rows_by_attr(rows, attr, exclude_spec) + if scope_paths is None: + return rows + return [r for r in rows if getattr(r, attr, None) in scope_paths] matched_modules: set[str] = set() if module_targets: module_set = set(module_targets) @@ -1824,7 +1845,7 @@ def bounded(rows: list[Any], label: str, *, cap: int | None = None) -> list[Any] test_finding_rows: list[Any] = [] test_findings_total = 0 if scoped: - finding_rows = filter_rows_by_attr( + finding_rows = in_scope_rows( list( ( await session.execute( @@ -1838,7 +1859,6 @@ def bounded(rows: list[Any], label: str, *, cap: int | None = None) -> list[Any] .all() ), "file_path", - exclude_spec, ) lead_rows: list[Any] = finding_rows emitted = _rank_emitted( @@ -1891,7 +1911,7 @@ def bounded(rows: list[Any], label: str, *, cap: int | None = None) -> list[Any] # ``lead_rows`` stays the unfiltered open set: it feeds the per-file # leads and the performance KPI, neither of which should change # because the caller asked to *see* one dimension. - lead_rows = filter_rows_by_attr(lite_rows, "file_path", exclude_spec) + lead_rows = in_scope_rows(lite_rows) emitted = _rank_emitted( [r for r in lead_rows if _in_dimensions(r, ranked_dimensions)] ) @@ -1993,8 +2013,6 @@ def bounded(rows: list[Any], label: str, *, cap: int | None = None) -> list[Any] # Dashboard perf headline: coverage (how much of the analyzed code the # perf pass ran on) + open performance-finding count. Both feed ``kpis`` # alone, so a projection that drops kpis skips the language-map read. - # The same map answers "how much of this headline is non-code" — one - # read, two KPIs. if not scoped and wants("kpis"): lang_by_path = await get_file_language_map(session, repository.id) perf_coverage = coverage_for_metrics(all_metrics, lang_by_path) @@ -2009,7 +2027,7 @@ def bounded(rows: list[Any], label: str, *, cap: int | None = None) -> list[Any] # ~10k rows the narrow pass above exists to avoid. accuracy_rows: list[Any] = [] if "accuracy" in include_set and not scoped: - accuracy_rows = filter_rows_by_attr( + accuracy_rows = in_scope_rows( list( ( await session.execute( @@ -2022,7 +2040,6 @@ def bounded(rows: list[Any], label: str, *, cap: int | None = None) -> list[Any] .all() ), "file_path", - exclude_spec, ) # Structured refactoring plans (Extract Class, ...) — loaded only when @@ -2041,14 +2058,13 @@ def bounded(rows: list[Any], label: str, *, cap: int | None = None) -> list[Any] or ({"performance", "refactoring"} <= include_set and wants("recommendation_lede")) ) if plans_requested and not nothing_resolved: - refactoring_rows = filter_rows_by_attr( + refactoring_rows = in_scope_rows( await get_refactoring_suggestions( session, repository.id, file_paths=list(effective_targets) if scoped else None, ), "file_path", - exclude_spec, ) refactoring_recommendations = await hydrate_recommendations( session, @@ -2098,7 +2114,7 @@ def bounded(rows: list[Any], label: str, *, cap: int | None = None) -> list[Any] coverage_rows: list[Any] = [] coverage_summary: dict[str, Any] = {} if "coverage" in include_set and not nothing_resolved: - coverage_rows = filter_rows_by_attr( + coverage_rows = in_scope_rows( # ``effective_targets``, not ``targets`` — a raw ``module:foo`` # target is not a file path and matched nothing here. # @@ -2114,11 +2130,15 @@ def bounded(rows: list[Any], label: str, *, cap: int | None = None) -> list[Any] include_covered_lines=scoped, ), "file_path", - exclude_spec, ) - # coverage_summary is a repo-wide stored aggregate, not recomputed - # here; the per-file rows above are exclude-filtered. - coverage_summary = await get_coverage_summary(session, repository.id) + # A repo-wide stored aggregate, not recomputed here, so it cannot + # describe a narrowed population. Omitted rather than served beside + # per-file rows that no longer match it; the rows themselves stay. + coverage_summary = ( + {} + if reported_scope == "production" + else await get_coverage_summary(session, repository.id) + ) # Per-file process/people/topology signals for targeted files — the # same join the file-detail drawer and REST breakdown use, so an agent @@ -2157,7 +2177,11 @@ def bounded(rows: list[Any], label: str, *, cap: int | None = None) -> list[Any] # file" context for agents). snapshots: list[Any] = [] if "trend" in include_set or (scoped and wants("trends")): - snapshots = await list_health_snapshots(session, repository.id, limit=20) + # Read through the same scope as the KPIs, or the two halves of one + # response would disagree about which files they describe. + snapshots = project_scope( + await list_health_snapshots(session, repository.id, limit=20), reported_scope + ) # Dominant-cause lead per file. Targeted mode wants one per target, so # the reduction runs over the whole (small) scoped set. Dashboard mode @@ -2234,20 +2258,17 @@ def bounded(rows: list[Any], label: str, *, cap: int | None = None) -> list[Any] if source: plan_biomarkers_by_path.setdefault(path, set()).add(source) - # KPIs deliberately keep test files in. Excluding them is not a display - # choice, it is a scoring change: measured across this workspace, dropping + # KPIs keep test files in by default, and ``scope`` is what changes that. + # The default is not arbitrary: measured across this workspace, dropping # test material moves NLOC-weighted ``average_health`` 7.52 -> 6.87 here, - # 7.07 -> 6.27 on the backend repo and 7.59 -> 7.46 on the frontend. Test - # files score *better* than production code, so excluding them would make - # every repo's headline drop overnight with no defect having been found. - # The calibrated numbers stay where they are; the split above is about which - # findings compete for a ranked list, not about what the score means. + # 7.07 -> 6.27 on the backend repo and 7.59 -> 7.46 on the frontend. Tests + # score better than production code, so a narrowed number is a lower number + # with no defect having been found — a caller should ask for it knowingly. kpis = _compute_kpis( metric_rows if scoped else all_metrics, hotspot_paths=hotspot_paths, performance_findings=perf_findings_count, coverage=perf_coverage, - lang_by_path=lang_by_path, ) if scoped: @@ -2379,6 +2400,7 @@ def bounded(rows: list[Any], label: str, *, cap: int | None = None) -> list[Any] else {} ), "mode": "dashboard", + "scope": reported_scope, "kpis": kpis, "distribution": health_distribution(all_metrics), # Where the gap to Healthy concentrates — the "few files, not the @@ -2551,7 +2573,10 @@ def bounded(rows: list[Any], label: str, *, cap: int | None = None) -> list[Any] if "trend" in include_set: summary = diff_snapshots(snapshots) + narrowed = reported_scope == "production" recent = recent_kpis(snapshots, limit=10) + if narrowed: + recent = drop_unscoped_fields(recent) alerts = [ { "kind": a.kind, @@ -2560,16 +2585,23 @@ def bounded(rows: list[Any], label: str, *, cap: int | None = None) -> list[Any] "baseline": a.baseline, "delta": a.delta, "message": a.message, + "driver": a.driver, + "structure_delta": a.structure_delta, + "history_delta": a.history_delta, } for a in summary.alerts ] + # The hotspot pair describes the whole repository whatever the scope, + # since only the average was snapshotted for both populations. result["trend"] = { - "current_hotspot_health": summary.current_hotspot_health, + "current_hotspot_health": None if narrowed else summary.current_hotspot_health, "current_average_health": summary.current_average_health, - "previous_hotspot_health": summary.previous_hotspot_health, + "previous_hotspot_health": None if narrowed else summary.previous_hotspot_health, "previous_average_health": summary.previous_average_health, - "hotspot_delta": summary.hotspot_delta, + "hotspot_delta": None if narrowed else summary.hotspot_delta, "average_delta": summary.average_delta, + "current_structure_deduction": summary.current_structure_deduction, + "current_history_deduction": summary.current_history_deduction, "alerts": bounded(alerts, "trend.alerts"), "alerts_total": len(alerts), "alerts_emitted": min(len(alerts), limit), diff --git a/packages/server/src/repowise/server/routers/code_health/files_routes.py b/packages/server/src/repowise/server/routers/code_health/files_routes.py index 34b47a98d..c85929490 100644 --- a/packages/server/src/repowise/server/routers/code_health/files_routes.py +++ b/packages/server/src/repowise/server/routers/code_health/files_routes.py @@ -16,6 +16,7 @@ from ._router import router from .breakdown import _score_breakdown_from_findings from .loaders import _attach_symbol_ids, _load_file_signals +from .scope import ScopeQuery, narrow from .serializers import ( _file_signals_to_dict, _file_trend_to_dict, @@ -58,11 +59,13 @@ async def list_health_files( "narrows the finding read that produces them." ), ), + scope: str = ScopeQuery, session: AsyncSession = Depends(get_db_session), ) -> dict: if sort not in _SORT_FIELDS: sort = "score" metrics = await crud.get_health_metrics(session, repo_id) + (metrics,) = narrow(scope, metrics) hotspot_paths: set[str] = set() if only_hotspots: diff --git a/packages/server/src/repowise/server/routers/code_health/map_routes.py b/packages/server/src/repowise/server/routers/code_health/map_routes.py index c44567518..e6b50acc7 100644 --- a/packages/server/src/repowise/server/routers/code_health/map_routes.py +++ b/packages/server/src/repowise/server/routers/code_health/map_routes.py @@ -21,6 +21,7 @@ ) from ._router import router +from .scope import ScopeQuery @router.get("/api/repos/{repo_id}/health/map") @@ -35,10 +36,11 @@ async def get_health_map( "them so the cap can never push the subject off its own map." ), ), + scope: str = ScopeQuery, session: AsyncSession = Depends(get_db_session), ) -> dict[str, Any]: """One bounded field plus the exact scope of what the cap left out.""" feed = await HealthMapService(session, repo_id).feed( - cap=cap, active=parse_active(active) + cap=cap, active=parse_active(active), scope=scope ) return feed.payload() diff --git a/packages/server/src/repowise/server/routers/code_health/overview_routes.py b/packages/server/src/repowise/server/routers/code_health/overview_routes.py index 43d889866..9c67734a9 100644 --- a/packages/server/src/repowise/server/routers/code_health/overview_routes.py +++ b/packages/server/src/repowise/server/routers/code_health/overview_routes.py @@ -23,6 +23,7 @@ from ._router import router from .loaders import _attach_symbol_ids +from .scope import ScopeQuery, narrow from .serializers import _finding_to_dict, _leads_by_file, _metric_to_dict @@ -49,6 +50,7 @@ def _resolve_last_indexed_at( async def health_overview( repo_id: str, limit: int = Query(20, ge=1, le=200), + scope: str = ScopeQuery, session: AsyncSession = Depends(get_db_session), ) -> dict: """KPIs + lowest-scoring files + per-module rollup + meta.""" @@ -62,22 +64,17 @@ async def health_overview( # two — so the route was paying for each of them twice per request. metrics = await crud.get_health_metrics(session, repo_id) findings = await crud.get_health_findings(session, repo_id) + metrics, findings = narrow(scope, metrics, findings) summary = await crud.get_health_summary( session, repo_id, metrics=metrics, findings=findings ) - # Hotspot health is recomputed from the metrics already loaded above rather - # than read off the latest snapshot. The snapshot was described here as - # authoritative, and it is not: ``repowise update`` re-scores health and - # calls ``save_health_metrics`` without ``save_health_snapshot`` - # (``update_cmd/persistence.py``), so after any update this route served a - # figure from the previous full index while every other number on the page - # came from the fresh rows. Measured stale on 3 of 42 local indexes, this - # repo among them (4.62 against 5.08 live) — a lower bound, since a corpus - # of frozen clones mostly has nothing to have gone stale against. - # - # It costs no query: ``metrics`` is already in hand, and the hotspot path - # set is one scalar column. The snapshot is still read, for ``taken_at``. + # Recomputed from the metrics already loaded rather than read off the latest + # snapshot: a snapshot is a point in time, and this page's other numbers all + # come from the live rows, so reading it here would make one figure older + # than its neighbours. It costs no query — ``metrics`` is in hand and the + # hotspot path set is one scalar column. The snapshot is still read, for + # ``taken_at``, and for ``scope`` it would be the wrong population anyway. snapshot = await crud.get_health_snapshot_headline(session, repo_id) hotspot_paths = await crud.get_hotspot_file_paths(session, repo_id) hotspot_health_value = hotspot_health(metrics, hotspot_paths) diff --git a/packages/server/src/repowise/server/routers/code_health/refactoring_routes.py b/packages/server/src/repowise/server/routers/code_health/refactoring_routes.py index 0c7c59fd9..77144efb3 100644 --- a/packages/server/src/repowise/server/routers/code_health/refactoring_routes.py +++ b/packages/server/src/repowise/server/routers/code_health/refactoring_routes.py @@ -18,6 +18,7 @@ from repowise.server.schemas import HealthWorkQueueResponse from ._router import router +from .scope import ScopeQuery, narrow _SEVERITY_ORDER = {"low": 0, "medium": 1, "high": 2, "critical": 3} @@ -65,6 +66,7 @@ async def health_work_queue( sort: str = Query( "impact_per_effort", pattern="^(impact_per_effort|total_impact|score|finding_count)$" ), + scope: str = ScopeQuery, session: AsyncSession = Depends(get_db_session), ) -> dict: """Health work items ranked by impact / effort. @@ -82,8 +84,9 @@ async def health_work_queue( raise HTTPException(status_code=404, detail="Repository not found") metrics = await crud.get_health_metrics(session, repo_id) - metric_by_path = {m.file_path: m for m in metrics} findings = await crud.get_health_findings(session, repo_id) + metrics, findings = narrow(scope, metrics, findings) + metric_by_path = {m.file_path: m for m in metrics} by_file: dict[str, list[Any]] = {} for f in findings: diff --git a/packages/server/src/repowise/server/routers/code_health/scope.py b/packages/server/src/repowise/server/routers/code_health/scope.py new file mode 100644 index 000000000..40306ea9f --- /dev/null +++ b/packages/server/src/repowise/server/routers/code_health/scope.py @@ -0,0 +1,34 @@ +"""Narrowing a code-health response to one half of the repository.""" + +from __future__ import annotations + +from typing import Any + +from fastapi import Query + +from repowise.core.analysis.health.scope import DEFAULT_SCOPE, SCOPES, parse_scope + +ScopeQuery = Query( + DEFAULT_SCOPE, + description=( + "Which files to report on: 'all' (production and tests) or 'production'. " + "Tests score higher than production code, so narrowing lowers every figure " + "without a defect having been found." + ), + pattern=f"^({'|'.join(SCOPES)})$", +) + + +def narrow( + scope: str, metrics: list[Any], *keyed_by_path: list[Any] +) -> tuple[list[Any], ...]: + """Filter *metrics* to *scope*, then everything else to the files it kept. + + Metric rows carry ``is_test``, so they answer the question directly. + Findings and their kin only carry a path, so they follow the metrics. + """ + if parse_scope(scope) != "production": + return (metrics, *keyed_by_path) + kept = [m for m in metrics if not m.is_test] + paths = {m.file_path for m in kept} + return (kept, *([r for r in rows if r.file_path in paths] for rows in keyed_by_path)) diff --git a/packages/server/src/repowise/server/routers/code_health/serializers.py b/packages/server/src/repowise/server/routers/code_health/serializers.py index 96a0b7645..7245e6e99 100644 --- a/packages/server/src/repowise/server/routers/code_health/serializers.py +++ b/packages/server/src/repowise/server/routers/code_health/serializers.py @@ -6,6 +6,7 @@ from typing import Any from repowise.core.analysis.health.models import primary_finding +from repowise.core.analysis.health.scoring import unclamped_score from repowise.core.analysis.health.signals import FileSignals from repowise.core.analysis.health.trends import FileTrend @@ -122,6 +123,14 @@ def _metric_to_dict( out["performance_findings"] = perf_findings out["performance_analyzed"] = perf_analyzed if not summary: + # The headline's two halves, and the score the file would carry without + # the floor. A file pinned at 1.0 reads 1.0 for months while real work + # lands on it; the unclamped number is the only place that shows. + structure = _round_opt(getattr(m, "structure_deduction", None)) + history = _round_opt(getattr(m, "history_deduction", None)) + out["structure_deduction"] = structure + out["history_deduction"] = history + out["unclamped_score"] = unclamped_score(structure, history) # Dominant-cause lead + pre-clamp magnitude (null when findings weren't # loaded for this row, or the file is clean). Additive; readers degrade. out["primary_biomarker"] = lead.get("primary_biomarker") if lead else None diff --git a/packages/server/src/repowise/server/routers/code_health/trends_routes.py b/packages/server/src/repowise/server/routers/code_health/trends_routes.py index 17dc9747a..cde60bf13 100644 --- a/packages/server/src/repowise/server/routers/code_health/trends_routes.py +++ b/packages/server/src/repowise/server/routers/code_health/trends_routes.py @@ -7,12 +7,20 @@ from fastapi import Depends, HTTPException, Query from sqlalchemy.ext.asyncio import AsyncSession -from repowise.core.analysis.health.trends import diff_snapshots, file_trend, recent_kpis +from repowise.core.analysis.health.scope import parse_scope +from repowise.core.analysis.health.trends import ( + diff_snapshots, + drop_unscoped_fields, + file_trend, + project_scope, + recent_kpis, +) from repowise.core.persistence import crud from repowise.server.deps import get_db_session from repowise.server.schemas import FileHealthTrendResponse, HealthTrendResponse from ._router import router +from .scope import ScopeQuery from .serializers import _file_trend_to_dict # How many per-file movements the trend response carries. Paired with @@ -44,13 +52,18 @@ async def file_health_trend( async def health_trend( repo_id: str, limit: int = Query(20, ge=1, le=50), + scope: str = ScopeQuery, session: AsyncSession = Depends(get_db_session), ) -> dict: repo = await crud.get_repository(session, repo_id) if repo is None: raise HTTPException(status_code=404, detail="Repository not found") - snapshots = await crud.list_health_snapshots(session, repo_id) + narrowed = parse_scope(scope) == "production" + snapshots = project_scope(await crud.list_health_snapshots(session, repo_id), scope) summary = diff_snapshots(snapshots) + # Snapshots record per-file scores for the whole repository, so a narrowed + # response has to drop the test files from the movement list too. + test_paths = await crud.get_test_file_paths(session, repo_id) if narrowed else set() # Per-file delta from the last two snapshots. file_deltas: list[dict] = [] @@ -64,7 +77,7 @@ async def health_trend( for p in all_paths: before = prev.get(p) after = cur.get(p) - if before is None or after is None: + if before is None or after is None or p in test_paths: continue d = round(float(after) - float(before), 2) if d == 0: @@ -80,14 +93,20 @@ async def health_trend( file_deltas.sort(key=lambda r: (-abs(r["delta"]), r["file_path"])) return { - "history": recent_kpis(snapshots, limit=limit), + "history": ( + drop_unscoped_fields(recent_kpis(snapshots, limit=limit)) + if narrowed + else recent_kpis(snapshots, limit=limit) + ), "summary": { - "current_hotspot_health": summary.current_hotspot_health, + "current_hotspot_health": None if narrowed else summary.current_hotspot_health, "current_average_health": summary.current_average_health, - "previous_hotspot_health": summary.previous_hotspot_health, + "previous_hotspot_health": None if narrowed else summary.previous_hotspot_health, "previous_average_health": summary.previous_average_health, - "hotspot_delta": summary.hotspot_delta, + "hotspot_delta": None if narrowed else summary.hotspot_delta, "average_delta": summary.average_delta, + "current_structure_deduction": summary.current_structure_deduction, + "current_history_deduction": summary.current_history_deduction, }, "alerts": [ { @@ -97,6 +116,9 @@ async def health_trend( "baseline": a.baseline, "delta": a.delta, "message": a.message, + "driver": a.driver, + "structure_delta": a.structure_delta, + "history_delta": a.history_delta, } for a in summary.alerts ], @@ -105,4 +127,5 @@ async def health_trend( # of presenting a truncated list as the whole story. "file_deltas_total": len(file_deltas), "snapshot_count": len(snapshots), + "scope": scope, } diff --git a/packages/server/src/repowise/server/schemas/code_health.py b/packages/server/src/repowise/server/schemas/code_health.py index c2982c1d0..f93ebbfd1 100644 --- a/packages/server/src/repowise/server/schemas/code_health.py +++ b/packages/server/src/repowise/server/schemas/code_health.py @@ -87,19 +87,26 @@ class HealthTrendKpiRow(BaseModel): """One snapshot in the repo-level history, newest first.""" taken_at: str | None = None - hotspot_health: float + #: ``None`` under a narrowed scope, which recorded only the average. + hotspot_health: float | None = None average_health: float worst_performer_path: str | None = None worst_performer_score: float | None = None class HealthTrendSummary(BaseModel): - current_hotspot_health: float + #: ``None`` under a narrowed scope: only the average was recorded for both + #: populations, and a repo-wide hotspot figure under a production label + #: would describe files the rest of the response has dropped. + current_hotspot_health: float | None = None current_average_health: float previous_hotspot_health: float | None = None previous_average_health: float | None = None hotspot_delta: float | None = None average_delta: float | None = None + #: The newest reading's two halves, in deduction points. + current_structure_deduction: float | None = None + current_history_deduction: float | None = None class HealthTrendAlert(BaseModel): @@ -109,6 +116,10 @@ class HealthTrendAlert(BaseModel): baseline: float | None = None delta: float message: str + #: Which half of the headline moved, and each half's share of ``delta``. + driver: str | None = None + structure_delta: float | None = None + history_delta: float | None = None class HealthFileDelta(BaseModel): @@ -129,6 +140,8 @@ class HealthTrendResponse(BaseModel): #: The count behind the slice, so the UI can say "N of M". file_deltas_total: int = 0 snapshot_count: int = 0 + #: Which half of the repository these figures describe. + scope: str = "all" class HealthBadgeResponse(BaseModel): diff --git a/packages/server/src/repowise/server/services/health_map.py b/packages/server/src/repowise/server/services/health_map.py index a8735bb9d..8d7a27ad9 100644 --- a/packages/server/src/repowise/server/services/health_map.py +++ b/packages/server/src/repowise/server/services/health_map.py @@ -21,6 +21,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from repowise.core.analysis.health.perf.coverage import supported_perf_languages +from repowise.core.analysis.health.scope import DEFAULT_SCOPE, parse_scope from repowise.core.persistence import crud __all__ = [ @@ -57,9 +58,12 @@ class HealthMapFeed: recovery: dict[str, Any] modules: list[dict[str, Any]] = field(default_factory=list) performance: dict[str, Any] | None = None + #: Which half of the repository the field describes. + scope: str = DEFAULT_SCOPE def payload(self) -> dict[str, Any]: return { + "scope": self.scope, "files": self.files, "cap": self.cap, "shown": self.shown, @@ -99,7 +103,11 @@ def __init__(self, session: AsyncSession, repository_id: str) -> None: self._repository_id = repository_id async def feed( - self, *, cap: int = DEFAULT_MAP_CAP, active: tuple[str, ...] = () + self, + *, + cap: int = DEFAULT_MAP_CAP, + active: tuple[str, ...] = (), + scope: str = DEFAULT_SCOPE, ) -> HealthMapFeed: session, repo_id = self._session, self._repository_id metrics = await crud.get_health_metrics(session, repo_id) @@ -108,6 +116,12 @@ async def feed( summary = await crud.get_performance_summary(session, repo_id) perf_languages = supported_perf_languages() + scope_narrowed = parse_scope(scope) == "production" + if scope_narrowed: + metrics = [m for m in metrics if not m.is_test] + kept = {m.file_path for m in metrics} + rollups = [r for r in rollups if r.file_path in kept] + by_path = {m.file_path: m for m in metrics} # A zero-NLOC file cannot be sized, and the map drops it on arrival. # Counting it as eligible would promise a node that never appears. @@ -185,7 +199,15 @@ def admit(path: str) -> bool: "raise_cap": f"cap accepts up to {MAX_MAP_CAP}.", }, modules=self._modules(drawn, burden), - performance=self._performance_block(rollups, summary, len(performance_eligible)), + # The stored performance summary is a repo-wide aggregate and there + # is no narrowed copy of it, so a narrowed field omits the block + # rather than serving repo-wide totals beside production-only rows. + performance=( + None + if scope_narrowed + else self._performance_block(rollups, summary, len(performance_eligible)) + ), + scope=DEFAULT_SCOPE if not scope_narrowed else "production", ) def _row( diff --git a/packages/types/src/generated/http.ts b/packages/types/src/generated/http.ts index a75ed634e..6cc6c147d 100644 --- a/packages/types/src/generated/http.ts +++ b/packages/types/src/generated/http.ts @@ -1543,12 +1543,15 @@ export interface HealthTrendAlert { baseline?: number | null; delta: number; message: string; + driver?: string | null; + structure_delta?: number | null; + history_delta?: number | null; } /** One snapshot in the repo-level history, newest first. */ export interface HealthTrendKpiRow { taken_at?: string | null; - hotspot_health: number; + hotspot_health?: number | null; average_health: number; worst_performer_path?: string | null; worst_performer_score?: number | null; @@ -1561,15 +1564,18 @@ export interface HealthTrendResponse { file_deltas?: HealthFileDelta[]; file_deltas_total?: number; snapshot_count?: number; + scope?: string; } export interface HealthTrendSummary { - current_hotspot_health: number; + current_hotspot_health?: number | null; current_average_health: number; previous_hotspot_health?: number | null; previous_average_health?: number | null; hotspot_delta?: number | null; average_delta?: number | null; + current_structure_deduction?: number | null; + current_history_deduction?: number | null; } /** One file in the triage queue, ranked by impact over effort. */ diff --git a/packages/types/src/health.ts b/packages/types/src/health.ts index 732f05906..e920405a4 100644 --- a/packages/types/src/health.ts +++ b/packages/types/src/health.ts @@ -48,9 +48,18 @@ export const HEALTH_DIMENSIONS: readonly HealthDimension[] = [ "performance", ] as const; +/** + * Which half of a repository a health figure describes. Tests score higher + * than production code, so narrowing lowers every figure without a defect + * having been found — `all` is the default for that reason. + */ +export type HealthScope = "all" | "production"; + +export const HEALTH_SCOPES: readonly HealthScope[] = ["all", "production"] as const; + /** Display labels for the dimensions surfaced today. */ export const HEALTH_DIMENSION_LABEL: Record = { - defect: "Defect risk", + defect: "Code health", maintainability: "Maintainability", performance: "Performance", }; @@ -179,6 +188,20 @@ export interface HealthFileMetric { defect_score?: number | null; maintainability_score?: number | null; performance_score?: number | null; + /** + * The defect deduction split into the half a rewrite can move (code shape) + * and the half only time can (git history). They sum to the total deduction, + * so `unclamped_score` is `10 - structure - history` — the only number that + * moves for a file held at the score floor. Absent on older payloads. + */ + structure_deduction?: number | null; + history_deduction?: number | null; + unclamped_score?: number | null; + /** + * Test material, decided at ingestion. Drives the production/all scope + * without re-deriving the answer from the path on every surface. + */ + is_test?: boolean; /** * Open performance-risk findings on this file. The performance lens on the * code-health map colors by this count (+ `performance_analyzed`), not by the @@ -779,18 +802,23 @@ export interface FileHealthTrend { export interface HealthTrendResponse { history: Array<{ taken_at: string | null; - hotspot_health: number; + /** `null` under a narrowed scope, which recorded only the average. */ + hotspot_health: number | null; average_health: number; worst_performer_path: string | null; worst_performer_score: number | null; }>; summary: { - current_hotspot_health: number; + /** `null` under a narrowed scope: only the average covers both populations. */ + current_hotspot_health: number | null; current_average_health: number; previous_hotspot_health: number | null; previous_average_health: number | null; hotspot_delta: number | null; average_delta: number | null; + /** The newest reading's two halves, in deduction points. */ + current_structure_deduction?: number | null; + current_history_deduction?: number | null; }; alerts: Array<{ kind: string; @@ -799,6 +827,15 @@ export interface HealthTrendResponse { baseline: number | null; delta: number; message: string; + /** + * Which half of the headline moved, and how far each half moved in score + * points. These are changes in mean deduction, so they sum to `delta` only + * while no file sits at the score floor. `null` on the hotspot metric, + * whose halves are not snapshotted, and on histories predating the split. + */ + driver?: "structure" | "history" | null; + structure_delta?: number | null; + history_delta?: number | null; }>; /** Largest movements first, in either direction, capped server-side. */ file_deltas: Array<{ @@ -813,6 +850,8 @@ export interface HealthTrendResponse { */ file_deltas_total?: number; snapshot_count: number; + /** Which half of the repository these figures describe. */ + scope?: HealthScope; } /* ------------------------------------------------------------------ * diff --git a/packages/ui/src/chat/artifacts.tsx b/packages/ui/src/chat/artifacts.tsx index 245d44b72..c6796f063 100644 --- a/packages/ui/src/chat/artifacts.tsx +++ b/packages/ui/src/chat/artifacts.tsx @@ -1017,7 +1017,8 @@ export function HealthRenderer({ data }: { data: Record }) { ["hotspot health", kpis.hotspot_health ?? data.hotspot_health], ["maintainability", kpis.maintainability_average ?? data.maintainability_average], ["performance", kpis.performance_average ?? data.performance_average], - ["code-only health", kpis.average_health_code_only ?? data.average_health_code_only], + ["structure deduction", kpis.structure_average ?? data.structure_average], + ["history deduction", kpis.history_average ?? data.history_average], ["worst performer", kpis.worst_performer_score ?? data.worst_performer_score], ].filter((entry): entry is [string, unknown] => entry[1] !== undefined && entry[1] !== null); const rows = findings.length > 0 ? findings : files; diff --git a/packages/ui/src/health/trend-chart.tsx b/packages/ui/src/health/trend-chart.tsx index 620530a53..4efc051ba 100644 --- a/packages/ui/src/health/trend-chart.tsx +++ b/packages/ui/src/health/trend-chart.tsx @@ -4,7 +4,8 @@ import { formatDate } from "../lib/format"; export interface TrendSeriesPoint { taken_at: string | null; - hotspot_health: number; + /** `null` under a narrowed scope, which records only the average. */ + hotspot_health: number | null; average_health: number; worst_performer_score: number | null; } @@ -78,7 +79,9 @@ export function TrendChart({ history, height = 220 }: TrendChartProps) { {history.map((p, i) => ( - + {p.hotspot_health != null ? ( + + ) : null} {p.worst_performer_score != null ? ( ) : null} diff --git a/packages/ui/src/health/trend-view.tsx b/packages/ui/src/health/trend-view.tsx index 2b108a1b1..054d18557 100644 --- a/packages/ui/src/health/trend-view.tsx +++ b/packages/ui/src/health/trend-view.tsx @@ -64,6 +64,11 @@ export function TrendView({ return `${formatDelta(delta)} vs. ${previous?.toFixed(1) ?? "—"}`; }; + // Absent under a narrowed scope: only the average was recorded for both + // populations, so a repo-wide hotspot figure would describe files this view + // has dropped. Say so rather than printing one. + const hotspot = summary.current_hotspot_health; + const stats: RibbonStat[] = [ { label: "Average health", @@ -76,10 +81,13 @@ export function TrendView({ }, { label: "Hotspot health", - value: summary.current_hotspot_health.toFixed(1), - valueColor: scoreTextColor(summary.current_hotspot_health), - sub: deltaSub(summary.hotspot_delta, summary.previous_hotspot_health), - ...(Math.abs(summary.hotspot_delta ?? 0) >= 0.05 + value: hotspot == null ? "—" : hotspot.toFixed(1), + ...(hotspot == null ? {} : { valueColor: scoreTextColor(hotspot) }), + sub: + hotspot == null + ? "not measured for this scope" + : deltaSub(summary.hotspot_delta, summary.previous_hotspot_health), + ...(hotspot != null && Math.abs(summary.hotspot_delta ?? 0) >= 0.05 ? { subColor: deltaColor(summary.hotspot_delta) } : {}), }, diff --git a/packages/vscode/src/core/webviewApi.ts b/packages/vscode/src/core/webviewApi.ts index 34edd5c60..597692685 100644 --- a/packages/vscode/src/core/webviewApi.ts +++ b/packages/vscode/src/core/webviewApi.ts @@ -229,10 +229,14 @@ export function createHostApi(ctx: RepowiseContext, epoch: () => number): HostAp openFindings: summary.open_findings, band: summary.band ?? null, hotspotDelta: trendVal?.summary?.hotspot_delta ?? null, + // A snapshot only carries a hotspot figure for the whole + // repository, so a reading without one is dropped rather than + // plotted at zero. history: (trendVal?.history ?? []) .slice() .reverse() - .map((p) => p.hotspot_health), + .map((p) => p.hotspot_health) + .filter((v): v is number => v !== null), } : null, counts: { diff --git a/tests/unit/server/mcp/test_health.py b/tests/unit/server/mcp/test_health.py index 85174d161..be8e28389 100644 --- a/tests/unit/server/mcp/test_health.py +++ b/tests/unit/server/mcp/test_health.py @@ -1868,50 +1868,104 @@ async def test_a_gradient_only_file_still_leads_with_the_gradient(setup_mcp, ses @pytest.mark.asyncio -async def test_kpis_report_how_much_of_the_headline_is_non_code(setup_mcp, session, populated_db): - """Markdown and JSON rows score a mechanical 10.0 and lift the average. - - No biomarker walks a non-code file, so its 10.0 means "nothing looked at - this" — the same fabricated-10.0 problem the perf pillar already surfaces - rather than hides. Measured on the live index: 233 of 3,314 rows are - non-code, 221 of them score exactly 10.0, and they lift ``average_health`` - from 7.31 to 7.47, so a repo can raise its score by adding documentation. - Surfaced rather than subtracted — ``average_health`` is what the badge, the - snapshots and the web UI read, and redefining it here alone would make this - tool disagree with all of them. +async def test_the_headline_averages_code_and_nothing_else(setup_mcp, session, populated_db): + """Prose and configuration carry no score, so they cannot lift the average. + + They used to: no marker walks a Markdown file, so its mechanical 10.0 meant + "nothing looked at this" and a repo could raise its score by writing + documentation. Measured on this repo, 293 of 4,064 rows were non-code and + they lifted the headline 6.92 -> 6.96. The analyzer now writes no row for + them at all, so there is no split to report and no ``non_code_files`` count + to explain away. """ from repowise.core.persistence.crud import save_health_metrics from repowise.server.mcp_server import get_health + await save_health_metrics( + session, + populated_db, + [{"file_path": "src/auth/service.py", "score": 4.0, "nloc": 100, "max_ccn": 9}], + ) + kpis = (await get_health())["kpis"] + assert kpis["file_count"] == 1 + assert kpis["average_health"] == 4.0 + assert "non_code_files" not in kpis + assert "average_health_code_only" not in kpis + + +@pytest.mark.asyncio +async def test_scope_narrows_every_figure_to_production(setup_mcp, session, populated_db): + """``scope`` is one filter over the whole response, not a per-block option.""" + from repowise.core.persistence.crud import save_health_metrics + from repowise.server.mcp_server import get_health + await save_health_metrics( session, populated_db, [ - {"file_path": "src/auth/service.py", "score": 4.0, "nloc": 100, "max_ccn": 9}, - # No graph node → no language → not in LANGUAGE_MAPS → non-code. - {"file_path": "docs/CHANGELOG.md", "score": 10.0, "nloc": 100, "max_ccn": 0}, + {"file_path": "src/auth/service.py", "score": 4.0, "nloc": 100, "is_test": False}, + {"file_path": "tests/test_auth.py", "score": 10.0, "nloc": 100, "is_test": True}, ], ) - kpis = (await get_health())["kpis"] - assert kpis["file_count"] == 2 - assert kpis["non_code_files"] == 1 - assert kpis["average_health"] == 7.0 - assert kpis["average_health_code_only"] == 4.0 + everything = await get_health() + assert everything["scope"] == "all" + assert everything["kpis"]["average_health"] == 7.0 + assert everything["kpis"]["file_count"] == 2 + + production = await get_health(scope="production") + assert production["scope"] == "production" + assert production["kpis"]["average_health"] == 4.0 + assert production["kpis"]["file_count"] == 1 + assert all( + row["file_path"] != "tests/test_auth.py" for row in production.get("worst_files", []) + ) @pytest.mark.asyncio -async def test_non_code_split_is_gated_on_the_language_read(setup_mcp, health_data): - """The split rides the language map ``kpis`` already reads — it adds no query. +async def test_a_narrowed_target_is_not_reported_as_config_excluded( + setup_mcp, session, populated_db +): + """``excluded`` means the repo's exclude config dropped the file. - So it appears only where that read happens: dashboard mode with ``kpis`` - surviving the projection. ``only=["directive"]`` stays the cheapest useful - call, and targeted mode (which serves no ``kpis`` block at all) is unchanged. + Narrowing to production drops files for an unrelated reason, and telling a + caller their config did it sends them to edit something irrelevant. """ + from repowise.core.persistence.crud import save_health_metrics from repowise.server.mcp_server import get_health - assert "kpis" not in await get_health(targets=["src/auth/service.py"]) - assert "kpis" not in await get_health(only=["directive"]) - assert "non_code_files" in (await get_health())["kpis"] + await save_health_metrics( + session, + populated_db, + [ + {"file_path": "src/auth/service.py", "score": 4.0, "nloc": 100, "is_test": False}, + {"file_path": "tests/test_auth.py", "score": 10.0, "nloc": 100, "is_test": True}, + ], + ) + result = await get_health(targets=["tests/test_auth.py"], scope="production") + reasons = {row["reason"] for row in result.get("unresolved", [])} + assert "excluded" not in reasons + + +@pytest.mark.asyncio +async def test_the_directive_names_a_cause_an_edit_can_remove(setup_mcp, health_data): + """``fix_first`` is an instruction, so its ``reason`` has to be actionable. + + History markers carry the largest caps, so the highest-leverage file is + often led by one. Naming it there tells the caller to fix something no edit + resolves; it belongs in ``watch``. + """ + from repowise.core.analysis.health.models import split_by_origin + from repowise.server.mcp_server import get_health + + directive = (await get_health())["directive"] + reason_marker = directive.get("reason") + assert directive["fix_first"] + watch = directive.get("watch") + if watch is not None: + marker = type("F", (), {"biomarker_type": watch["biomarker"]}) + code_shape, _ = split_by_origin([marker]) + assert not code_shape, "watch must carry a history marker, never a code-shape one" + assert watch["biomarker"] not in (reason_marker or "") @pytest.mark.asyncio