Make a halt record itself as a halt, not as a completion

exemptions.json

@@ -269,28 +269,28 @@
     },
     {
       "file": "src/bonfire/onboard/flow.py",
-      "symbol_or_line": "147",
+      "symbol_or_line": "run_front_door",
       "rule": "C901",
       "reason": "grandfathered at v1.0.1 cf-gate adoption 2026-06-22 (ratchet: BON-1757) — pre-existing under the kit ruff config; tracked for fix-forward",
       "approver": "BubbleGum mount (v1.0.1 gate-adoption), 2026-06-22 — pending Anta's blessing via PR #227"
     },
     {
       "file": "src/bonfire/onboard/flow.py",
-      "symbol_or_line": "147",
+      "symbol_or_line": "run_front_door",
       "rule": "PLR0915",
       "reason": "grandfathered at v1.0.1 cf-gate adoption 2026-06-22 (ratchet: BON-1757) — pre-existing under the kit ruff config; tracked for fix-forward",
       "approver": "BubbleGum mount (v1.0.1 gate-adoption), 2026-06-22 — pending Anta's blessing via PR #227"
     },
     {
       "file": "src/bonfire/onboard/flow.py",
-      "symbol_or_line": "239",
+      "symbol_or_line": "run_front_door",
       "rule": "BLE001",
       "reason": "Shutdown drain of cancelled asyncio tasks: the asyncio.CancelledError arm is load-bearing (deliberately swallows the cancel during drain); the broad arm catches a drained task's own arbitrary error.",
       "approver": "BubbleGum/Elegance-Law architectural blessing (BON-1757); Anta gates via PR merge"
     },
     {
       "file": "src/bonfire/onboard/flow.py",
-      "symbol_or_line": "239",
+      "symbol_or_line": "run_front_door",
       "rule": "S110",
       "reason": "grandfathered at v1.0.1 cf-gate adoption 2026-06-22 (ratchet: BON-1757) — pre-existing under the kit ruff config; tracked for fix-forward",
       "approver": "BubbleGum mount (v1.0.1 gate-adoption), 2026-06-22 — pending Anta's blessing via PR #227"
@@ -311,7 +311,7 @@
     },
     {
       "file": "src/bonfire/onboard/orchestrator.py",
-      "symbol_or_line": "106",
+      "symbol_or_line": "_run_one",
       "rule": "BLE001",
       "reason": "Per-scanner boundary: scanners are a pluggable open set; one scanner failing must not abort onboarding (log + count=0 + continue).",
       "approver": "BubbleGum/Elegance-Law architectural blessing (BON-1757); Anta gates via PR merge"

file-budget.json

@@ -145,9 +145,9 @@
     "src/bonfire/dispatch": 2869,
     "src/bonfire/engine": 1981,
     "src/bonfire/handlers": 3250,
-    "src/bonfire/onboard": 4416,
+    "src/bonfire/onboard": 4460,
     "tests/integration": 1702,
-    "tests/unit": 73410
+    "tests/unit": 73777
   },
   "package_raises": [
     {
@@ -197,6 +197,22 @@
       "lines": 339,
       "reason": "tests/integration measured EXACTLY its ceiling on origin/main (1363/1363), so the package had zero headroom and no pull request could add an integration test at all. What the 339 lines buy: tests/integration/test_run_checkpoints.py, eight tests that run the engine returned by build_default_engine. They have to live in tests/integration and they have to go through the composition root: this defect family -- Envelope.artifacts, review-verdict.json, costs.jsonl and now the checkpoint -- is invisible to unit tests by construction, because every unit test of the run path injects its own engine factory, so the wiring nothing exercises is the wiring nothing can catch. A test that assembles the object graph by hand re-implements the wiring under test and passes whether or not the product is wired. Both halves are rodded: removing the engine's write site turns 5 of the 8 red, and removing checkpoint_sink= from build_default_engine turns the same 4 red plus the wiring assertion, while the two tests that must not depend on the write stay green. Explicitly rejected: putting these in tests/unit, which would have been the same line count against a ceiling a second live lane needs this round -- and would have meant hand-building the engine, which is the measurement error this file exists to avoid. Also rejected: a purpose entry for the new file, which registers it out of the ratchet entirely rather than paying for it.",
       "approver": "Anta gates via PR merge -- raise surfaced in the PR body and in the lane report, not taken silently"
+    },
+    {
+      "package": "tests/unit",
+      "from": 73410,
+      "to": 73777,
+      "lines": 367,
+      "reason": "tests/unit measured 73409 against a ceiling of 73410 -- one line of headroom, the second consecutive lane to arrive at a ratchet sitting on its own measurement (the raise immediately below moved it 73199 -> 73410 and landed it right back on the number). What the 367 lines buy: tests/unit/test_failure_is_not_a_completion.py, twenty tests over four places where a failure recorded itself as a completion or gave a reason that was not the reason that occurred. The money-facing one is the cost ledger: a halted run and a run that finished one stage wrote rows differing ONLY in timestamp, so 'bonfire cost' -- the operator's record of what they were charged and why -- could report the spend but not whether it bought a finished run or a crash. The others are the XP penalty's reason, a crashed scanner reported to the browser as a clean scan of nothing, and every client-frame validation error reported as message_too_long. Each defect gets a regression test that fails on origin/main's behaviour AND a negative control asserting a genuine success still records as a success, so none of it can be satisfied by code that reports everything as broken; two further tests pin the on-disk migration (pre-field rows still load, and still aggregate through CostAnalyzer, without being fabricated into successes). Explicitly rejected: (1) shipping fewer tests -- the control rods ARE the deliverable and the negative controls are what stop the fix from being 'call everything a failure'; (2) parking the file in an unbudgeted tree, which costs zero budget and is dodging the ratchet rather than restructuring, and was rejected on the same grounds by the raise below; (3) a purpose entry, an exemption in all but name that would drop the file out of the ratchet permanently; (4) shrinking another test file to make room, which races two lanes live in this tree right now; (5) padding the ask to leave headroom for the next lane -- the number is the measurement, and a lane granting itself slack is how a ratchet dies. The file was tightened before the ask was sized (a shared sink helper and a shared legacy-row constant replaced duplicated setup, and two pairs of tests became parametrized), which took the measurement from 389 to 368.",
+      "approver": "Anta gates via PR merge -- raise surfaced in the PR body and in the lane report, not taken silently"
+    },
+    {
+      "package": "src/bonfire/onboard",
+      "from": 4416,
+      "to": 4460,
+      "lines": 44,
+      "reason": "src/bonfire/onboard measured EXACTLY 4416 against a ceiling of 4416 -- zero headroom, verified by a one-line probe file producing 'package src/bonfire/onboard is 4417 lines'. The 44 lines fix two records in this package that reported a failure as a success. In orchestrator.py a scanner that CRASHED emitted ScanComplete(item_count=0), identical to a scanner that ran clean and found nothing, so the browser said 'we scanned and found nothing' over a scan that had died; _run_one now returns (count, failed) and reports the real exception, and run_scan totals failed panels so an all-crashed run does not summarise as total_items=0. In flow.py every ValidationError on a client frame was reported as message_too_long -- including a frame with no text field at all and a frame whose text was a number -- sending the user to shorten a message that was never long; _frame_rejection now keeps message_too_long only for a genuine pydantic string_too_long and names the actual cause otherwise. protocol.py carries the three new fields, all defaulted to the success shape so an older producer's frame still parses. Explicitly rejected: (1) reporting the crash only to the log, which is where it already went and is not visible to the browser that is the actual consumer; (2) compressing docstrings elsewhere in the package to buy the lines back, which trades documentation for budget and measures as nothing anyway since the unit is max(physical, statements); (3) moving the fix out of onboard into an unbudgeted package, which would take the producer off the path that produces. The protocol.py docstrings WERE trimmed once for size before this number was taken.",
+      "approver": "Anta gates via PR merge -- raise surfaced in the PR body and in the lane report, not taken silently"
     }
   ]
 }

src/bonfire/cost/consumer.py

@@ -68,6 +68,7 @@ async def _on_pipeline_completed(self, event: PipelineCompleted) -> None:
             total_cost_usd=event.total_cost_usd,
             duration_seconds=event.duration_seconds,
             stages_completed=event.stages_completed,
+            outcome="completed",
         )
         self._append(record)
 
@@ -81,13 +82,25 @@ async def _on_pipeline_failed(self, event: PipelineFailed) -> None:
         — every failed session no longer looks instant with zero
         stages done, and downstream analyzers can compute meaningful
         success-rate / mean-time-to-halt over the ledger.
+
+        Carrying those two fields alone made the halt row a perfect
+        forgery of a success row: a run that died in the builder and a
+        run that finished one stage both wrote ``stages_completed=1``
+        and nothing else differed but ``timestamp``. ``outcome`` is the
+        field that separates them, and ``failed_stage`` /
+        ``error_message`` say WHICH failure it was — copied from the
+        event rather than inferred, so the ledger's reason is the
+        reason that actually occurred.
         """
         record = PipelineRecord(
             timestamp=event.timestamp,
             session_id=event.session_id,
             total_cost_usd=event.total_cost_usd,
             duration_seconds=event.duration_seconds,
             stages_completed=event.stages_completed,
+            outcome="failed",
+            failed_stage=event.failed_stage,
+            error_message=event.error_message,
         )
         self._append(record)
 

src/bonfire/cost/models.py

@@ -56,14 +56,37 @@ class DispatchRecord(BaseModel):
 
 
 class PipelineRecord(BaseModel):
-    """One pipeline completion with total cost."""
+    """One pipeline run with total cost, and how that run ENDED.
+
+    ``outcome`` exists because a halt and a completion used to write
+    byte-identical rows apart from ``timestamp``. A run that died in
+    the builder and a run that finished one stage both landed as
+    ``stages_completed=1``, so ``bonfire cost`` — the operator's record
+    of what they were charged and why — could report spend but could
+    not say whether the money bought a finished run or a crash.
+
+    ``failed_stage`` and ``error_message`` carry the reason across from
+    ``PipelineFailed`` rather than being re-derived, so the ledger
+    states the cause that actually occurred instead of an inferred one.
+
+    Migration: the default is ``"unknown"``, NOT ``"completed"``. Rows
+    written before this field existed genuinely do not record how the
+    run ended, and defaulting them to success would fabricate exactly
+    the history this defect corrupted. ``CostAnalyzer`` does not list
+    ``outcome`` in ``_PIPELINE_REQUIRED_FIELDS``, so those rows keep
+    validating and keep aggregating unchanged; they simply decline to
+    claim an outcome nobody recorded.
+    """
 
     type: Literal["pipeline"] = "pipeline"
     timestamp: float
     session_id: str
     total_cost_usd: float
     duration_seconds: float
     stages_completed: int
+    outcome: Literal["completed", "failed", "unknown"] = "unknown"
+    failed_stage: str | None = None
+    error_message: str | None = None
 
 
 class SessionCost(BaseModel):

src/bonfire/onboard/flow.py

@@ -90,6 +90,28 @@ class MessageTooLargeError(BrowserDisconnectedError):
     """
 
 
+def _frame_rejection(exc: ValidationError) -> ServerError:
+    """Name the ACTUAL reason a client frame was refused.
+
+    Every ``ValidationError`` on a ``user_message`` frame used to be
+    reported as ``message_too_long``. A frame with no ``text`` field at
+    all, and a frame whose ``text`` was a number, both told the user to
+    shorten a message that was never long — sending them to fix
+    something that is not broken. Only a genuine length violation
+    (pydantic ``string_too_long``) keeps that code; anything else is
+    reported as what it is.
+    """
+    if any(err["type"] == "string_too_long" for err in exc.errors()):
+        return ServerError(
+            code="message_too_long",
+            message="Message too long; please keep under 8 KiB.",
+        )
+    detail = "; ".join(
+        f"{'.'.join(str(p) for p in err['loc'])}: {err['msg']}" for err in exc.errors()
+    )
+    return ServerError(code="invalid_message", message=f"Message rejected — {detail}")
+
+
 async def dispatch_user_message(
     data: dict[str, Any],
     *,
@@ -105,7 +127,9 @@ async def dispatch_user_message(
     - Non-``user_message`` frames are ignored.
     - Overlong payloads (>``MAX_USER_MESSAGE_LEN``) trigger a
       ``server_error`` frame with code ``message_too_long`` and never reach
-      the conversation analyzer.
+      the conversation analyzer. Frames rejected for any OTHER reason get
+      code ``invalid_message`` naming the real cause — see
+      :func:`_frame_rejection`.
     - ``ConversationCompleteError`` after the third answer is broadcast as
       a polite ``server_error`` and is NOT propagated.
     - Once the conversation is complete, ``conversation_done`` is signalled
@@ -120,13 +144,8 @@ async def dispatch_user_message(
 
     try:
         msg = UserMessage.model_validate(data)
-    except ValidationError:
-        await broadcast(
-            ServerError(
-                code="message_too_long",
-                message="Message too long; please keep under 8 KiB.",
-            )
-        )
+    except ValidationError as exc:
+        await broadcast(_frame_rejection(exc))
         return
 
     try:

src/bonfire/onboard/orchestrator.py

@@ -77,10 +77,11 @@ async def run_scan(
     await emit(ScanStart(panels=panel_names))
 
     tasks = [_run_one(panel, module, project_path, emit) for panel, module in scanners]
-    results: list[int] = await asyncio.gather(*tasks)
+    results: list[tuple[int, bool]] = await asyncio.gather(*tasks)
 
-    total = sum(results)
-    await emit(AllScansComplete(total_items=total))
+    total = sum(count for count, _ in results)
+    failed_panels = sum(1 for _, failed in results if failed)
+    await emit(AllScansComplete(total_items=total, failed_panels=failed_panels))
     return total
 
 
@@ -94,18 +95,29 @@ async def _run_one(
     module: ModuleType,
     project_path: Path,
     emit: Callable[[FrontDoorMessage], Awaitable[None]],
-) -> int:
-    """Execute a single scanner, catch failures, emit ScanComplete."""
+) -> tuple[int, bool]:
+    """Execute a single scanner, emit ScanComplete, report ``(count, failed)``.
+
+    A crashed scanner used to be indistinguishable from a clean scan
+    that found nothing: both emitted ``item_count=0`` and the browser
+    said "we scanned and found nothing" over a scanner that had died.
+    The exception is still swallowed — one broken scanner must not
+    abort the other five — but it is now REPORTED rather than erased.
+    """
 
     async def _narrow_emit(event: ScanUpdate) -> None:
         """Forward ScanUpdate from the scanner to the orchestrator emit."""
         await emit(event)
 
+    failed = False
+    error: str | None = None
     try:
         count = await module.scan(project_path, _narrow_emit)
-    except Exception:  # noqa: BLE001
+    except Exception as exc:  # noqa: BLE001
         _log.exception("Scanner %s failed", panel)
         count = 0
+        failed = True
+        error = f"{type(exc).__name__}: {exc}"
 
-    await emit(ScanComplete(panel=panel, item_count=count))
-    return count
+    await emit(ScanComplete(panel=panel, item_count=count, failed=failed, error=error))
+    return count, failed

src/bonfire/onboard/protocol.py

@@ -90,18 +90,31 @@ class ScanUpdate(FrontDoorMessage):
 
 
 class ScanComplete(FrontDoorMessage):
-    """One scan panel has finished."""
+    """One scan panel has stopped — successfully or otherwise.
+
+    ``failed``/``error`` separate a scanner that CRASHED from one that
+    ran clean and found nothing; both used to report only
+    ``item_count=0``. Both default to the success shape, so a frame
+    from an older producer still parses as a clean scan.
+    """
 
     type: Literal["scan_complete"] = "scan_complete"
     panel: str
     item_count: int
+    failed: bool = False
+    error: str | None = None
 
 
 class AllScansComplete(FrontDoorMessage):
-    """All scan panels have finished."""
+    """All scan panels have stopped.
+
+    ``failed_panels``: an all-crashed run used to summarise as
+    ``total_items=0``, identical to a clean scan of an empty project.
+    """
 
     type: Literal["all_scans_complete"] = "all_scans_complete"
     total_items: int
+    failed_panels: int = 0
 
 
 class ConversationStart(FrontDoorMessage):

src/bonfire/onboard/ui.html

@@ -139,6 +139,19 @@
     content: ' \2713';
     color: var(--green);
   }
+  /* A scanner that crashed. Deliberately NOT the completion tick: a
+     dead scan and an empty one both arrived as item_count=0 before. */
+  .scan-panel.failed {
+    border-color: var(--amber, #ffb000);
+    transition: border-color 0.5s ease;
+  }
+  .scan-panel.failed h2 {
+    color: var(--amber, #ffb000);
+  }
+  .scan-panel.failed h2::after {
+    content: ' \26A0';
+    color: var(--amber, #ffb000);
+  }
   @keyframes panel-pulse {
     0%, 100% { box-shadow: none; }
     50% { box-shadow: 0 0 8px rgba(0, 255, 65, 0.15); }
@@ -375,8 +388,8 @@
     switch (msg.type) {
       case 'scan_start': statusEl.textContent = 'scanning ' + msg.panels.length + ' systems...'; break;
       case 'scan_update': addScanItem(msg.panel, msg.label, msg.value, msg.detail || ''); break;
-      case 'scan_complete': markPanelComplete(msg.panel, msg.item_count); break;
-      case 'all_scans_complete': statusEl.textContent = msg.total_items + ' findings catalogued'; break;
+      case 'scan_complete': markPanelComplete(msg.panel, msg.item_count, msg.failed, msg.error); break;
+      case 'all_scans_complete': statusEl.textContent = summariseScans(msg.total_items, msg.failed_panels); break;
       case 'conversation_start': activateChat(); break;
       case 'falcor_message':
         if (msg.subtype === 'narration') { setNarration(msg.text); }
@@ -400,13 +413,27 @@
     el.appendChild(item);
   }
 
-  function markPanelComplete(panel, count) {
+  // A scanner that DIED must not render as one that found nothing.
+  // Both used to arrive as item_count=0; 'failed' is what separates them.
+  function markPanelComplete(panel, count, failed, error) {
     if (KNOWN_PANELS.indexOf(panel) === -1) return;
     var panelEl = document.querySelector('[data-panel="' + panel + '"]');
     if (!panelEl) return;
-    panelEl.classList.add('complete');
+    panelEl.classList.add(failed ? 'failed' : 'complete');
     var h2 = panelEl.querySelector('h2');
-    if (h2) h2.textContent += ' [' + count + ']';
+    if (!h2) return;
+    if (failed) {
+      h2.textContent += ' [scan failed]';
+      if (error) panelEl.setAttribute('title', error);
+    } else {
+      h2.textContent += ' [' + count + ']';
+    }
+  }
+
+  function summariseScans(total, failedPanels) {
+    var summary = total + ' findings catalogued';
+    if (failedPanels) summary += ' — ' + failedPanels + ' scan(s) failed';
+    return summary;
   }
 
   function setNarration(text) { enqueueType(narrationEl, text, 35); }