Skip to content

fix(server): stop dropping late practices when a review run hits its budget #1504

Description

@FelixTJDietrich

Outcome

A review run that hits its time budget records which practices it did not reach. No practice is silently
dropped, and the observation record is never biased by the order practices happen to be batched in.

Problem

pi-runner.mjs drives the practices of a review in area-ordered batches. When the hard watchdog fires:

  • the turn loop breaks — } catch (err) { … if (hardAborted) break; }
  • and the coverage gate that would have caught the gap is skipped — if (!hardAborted && allSlugs.length > 0) { … } (server/src/main/resources/agent/pi-runner.mjs:1195)

So the practices that sort last in area order are never evaluated, nothing records that they were
skipped, and the result is indistinguishable from a run in which they legitimately found nothing.

Two consequences:

  1. The record is biased, not merely incomplete. The missing observations are not a random sample —
    they are whatever the area ordering puts last. Every sampled audit (feat(webapp,server): accessible operator audit queue for emitted observations #1365, feat(server,webapp): per-practice resolved-audit support summaries #1366) and every
    reliability figure drawn from shadow data inherits that bias.
  2. It gets worse as the catalogue grows. The catalogue is expected to grow well past today's size,
    and a larger practice set makes hitting the budget the normal case rather than the exception.

What

  • Record an explicit, typed outcome for a practice a run did not reach, distinct from "reviewed and found
    nothing" and from "turned off". PracticeTraceOutcome already distinguishes NOT_ASSESSABLE from
    TURNED_OFF; add the not-reached case beside them and surface it in the per-artifact trace.
  • Run the coverage reconciliation on the abort path too, so a hard abort still produces a complete
    account of what was and was not evaluated.
  • Emit a per-run counter of evaluated-versus-eligible practices so the rate is observable rather than
    inferred.

Acceptance criteria

  • A run that aborts on the watchdog produces a typed record for every eligible practice it did not
    evaluate.
  • The per-artifact trace distinguishes not-reached from not-assessable and from turned-off.
  • A dashboard or log signal reports the evaluated-versus-eligible ratio per run.
  • A test aborts a run mid-batch and asserts that no eligible practice is missing from the account.

Out of scope

  • Choosing which practices to evaluate when the budget is tight. That is a separate selection policy and
    should not be designed under a bug fix; this issue only guarantees that whatever is skipped is recorded.

Metadata

Metadata

Assignees

No one assigned

    Labels

    application-serverSpring Boot server: APIs, business logic, databasebugSomething isn't workingpriority:criticalDrop everything - Loss of functionality or data

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions