Skip to content

stewards: shadow assessment module with a divergence log (BB-293) - #321

Merged
trotterdylan merged 2 commits into
mainfrom
bb/bb-293-shadow-assessment-module-divergence-log-thr_hb2w7k68b5
Nov 10, 2026
Merged

trotterdylan merged 2 commits into
mainfrom
bb/bb-293-shadow-assessment-module-divergence-log-thr_hb2w7k68b5

Conversation

@trotterdylan

@trotterdylan trotterdylan commented Nov 10, 2026 •

Copy link
Copy Markdown
Collaborator

Implements BB-293. Child 2 of BB-291. Adds a shadow-only assessment module to the stewards plugin: pure assess derives one answer per binding for the merge gate (with the decide-list mark and the recorded project approvals the approvals.ts path omits), the worker state (an explicit parked, distinct from running) and the wait liveness (wake source / idle / stall from the observable rows, not the report-time noWakeSource flag). compare runs the predicates it will replace and returns every disagreement; the reconcile stores the latest assessment per binding and logs each distinct divergence. Nothing reads the assessment to decide or render: a consumer is added only after the comparison has run clean in production for several days.

Merging this moves the bb-plugins pin in bb-gate and stages a restart for every developer's server (Restart now, or the 03:00–05:00 ET quiet window).

Classification

owner-reviewed: a store migration (plugins/stewards/store.ts) — the project's bigDecisions entry "A store migration" (bb-plugins/plugins/*/store.ts) is decide: project, and the rest of the change is the stewards plugin, which the same decide list gives the project steward. The project steward approves; this is not the owner's.

Any later BB-291 child that lets the assessment's merge-gate answer decide or replace prGate or the approvals gate is the user's decision under the no-self-widening rule (the "roles parsing, ready gate and escalation routing" entry), whatever this list says. This PR does not: gate.ts, approvals.ts, ready-gate.ts, escalations.ts and roles.ts are unedited, and the assessment only imports from them.

What changes

  • assessment.ts (new, pure): assess(binding, rows…) returns { gate, worker, liveness }; compare returns { binding, surface, old, new, inputs } divergences. No extra GitHub or Linear call: it reads the rows a reconcile already has in hand (the snapshot's pull.files supplies the decide-list mark).
  • store.ts: an append-only migration group ASSESSMENT_MIGRATIONS adds assessments (latest per binding) and assessment_divergences (one row per distinct binding/surface/old/new, with first/last seen and a count). No column renamed or altered; migrations.snapshot.json appends the four new hashes. purgeProject drops the new rows with the binding.
  • reconcile.ts: observedStall, the stall derived from the observable rows (the assessment's replacement for the report-time flag).
  • server.ts: the reconcile computes assess+compare on every binding inside a try/catch that can never change the reconcile result; an unchanged divergence bumps its count instead of re-logging; the log is pruned by both a row count and an age on each reconcile. New read-only bb stewards divergences [--binding] [--surface] [--json].
  • DESIGN.md: a short "The shadow assessment" note in the reconcile section.

Retirement list, per derivation

Derivation Predicate / flag / silent branch it will retire
assess.gate (merge gate, with decision and recorded project approvals) approvals.ts's gate construction prGate(pull, productGateOptions(b, pull)) in recordProjectDecision and approvePrPageAction, which omits the decide mark and the recorded approvals; and the approvePrPageAction refusal's ?? silent branch (gate?.missing ?? "the pull request has not been observed yet") that fires on a met gate. gateState's ready-gate pass is folded into gate.readyPassed.
assess.worker.state (explicit parked) workerInFlight / anyWorkerInFlight (a parked finished worker counted as busy) and workerRunning / runningWorkerReason (the same worker counted not mid-turn) in reconcile.ts; and idleInActiveStage's treatment of the parked worker.
assess.liveness (wake source / idle / stall) hasLiveWakeSource (wake-source.ts), idleInActiveStage and stuckDue (reconcile.ts) are folded into the one answer; the report-time noWakeSource flag (set at the steward_report path, stored by WAKE_SOURCE_MIGRATIONS) is replaced by observedStall recomputed from the rows.

The BB-292 divergences each appear as a fixture in assessment.test.ts whose divergence compare returns: the parked worker (workerInFlight / runningWorkerReason / idleInActiveStage / hasLiveWakeSource), the approvals.ts gate inputs, and the page refusal text. The BB-266 and bb-plugins#294 fixtures are covered, and every gate-met fixture carries a parked worker and a second open own pull request unless the test is about their absence.

One existing test file changed, mechanically: the two migration-order assertions in store.test.ts now name ASSESSMENT_MIGRATIONS as the newest group. The append-only rule forces this — a new group must be appended after STACKED_PULLS_MIGRATIONS, and those tests pin the last group — and no behavioural test changed. The characterization suite (characterization.test.ts) is untouched.

Testing

  • npx tsc --noEmit in plugins/stewards: pass.
  • npm test in plugins/stewards: pass (1576 tests, 34 files); CI's quality_checks (stewards) runs the same suite and is green.
  • assessment.test.ts: the pure derivations, the three BB-292 divergences, the BB-266 and deps: bump pnpm/action-setup from 6.1.0 to 6.1.1 in the github-actions group across 1 directory #294 fixtures, the store's latest-per-binding / dedupe / prune bounds, and the CLI formatter.
  • server.test.ts: a new end-to-end case that reconciles a binding, confirms the assessment is stored, and exercises bb stewards divergences with --binding, --surface and --json.
  • Not tested: no live page, webhook or GitHub path was exercised beyond the existing suites; the assessment is shadow and changes no surface output.

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Independent review running in bb thread @thread:thr_d7hihyjv4f.

— from bb thread thr_hb2w7k68b5

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Independent review running in bb thread @thread:thr_fh6fwzukit (the first reviewer, @thread:thr_d7hihyjv4f, was stopped by the plugin's spawn guard; respawned on Pi/GLM 5.3).

— from bb thread thr_hb2w7k68b5

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Review round 1 (GLM 5.3, at b85dd13)

Owner review required: yes — a store migration (plugins/stewards/store.ts, new assessments and assessment_divergences tables); decide list entry "A store migration" (bb-plugins/plugins/*/store.ts) is decide: project, and the stewards-plugin entry matches the rest of the diff, so the project steward approves.

The rest checks out against the ticket and the steward's binding rules: gate.ts, approvals.ts, ready-gate.ts, escalations.ts and roles.ts are unedited and assessment.ts only imports the predicates it shadows; recordShadowAssessment reads only rows the reconcile already has (decideForPull runs on the snapshot's files, no extra GitHub or Linear call); divergence inputs carry only ids, shas, states, booleans and timestamps; the migration group is appended after STACKED_PULLS_MIGRATIONS with migrations.snapshot.json appended-only (four new hashes, one comma); the prune is two-sided (age 7d, 500 rows) and tested; the three BB-292 divergences, the BB-266 shape and the #294 shape each have a fixture whose divergence (or agreement) compare records, matching #320's characterization; the only existing-test edit is the two mechanical migration-order assertions in store.test.ts; the liveWakeSourceFor split is behaviour-identical and no existing surface output changes.

Blockers

  1. plugins/stewards/server.ts:4072-4076 — the swallow half of criterion 2 ("a failure inside assessment is swallowed and logged, never changing reconcile output") has no test. Removing the try/catch around recordShadowAssessment fails no test, and the swallow is the safety property that makes the shadow module shippable: it is what guarantees a bug in assessment.ts or the new store writes can never break a reconcile. Easy to exercise with the existing harness: after setup(), drop the assessments table (the lazy prepared statement then throws) or otherwise force the write to fail, ring a doorbell, and assert the reconcile still completes with its normal wake/output — ideally asserting the stewards: shadow assessment failed warning was logged.

Tested

  • Ran: npm test (1575 passed, 34 files) and npx tsc --noEmit in plugins/stewards — both pass at b85dd13.
  • The PR body's "not in the CI matrix" is wrong: quality_checks (stewards) is in the plugins workflow's matrix and ran green on this PR.

— from bb thread thr_fh6fwzukit

@trotterdylan

trotterdylan commented Nov 10, 2026 •

Copy link
Copy Markdown
Collaborator Author

Round 1 answered.

  • Blocker 1 (the swallow half of criterion 2 has no test): fixed in bed18b8. server.test.ts now drops the assessments table before a doorbell, rings, and asserts the reconcile still stores the binding's snapshot and logs stewards: shadow assessment failed. I confirmed the test fails when the try/catch around recordShadowAssessment is removed.
  • PR body correction: the "not in the CI matrix" claim is fixed — quality_checks (stewards) is in the plugins matrix and runs the same suite, and the test count is now 1576.

— from bb thread thr_hb2w7k68b5

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Review round 2 (GLM 5.3, at bed18b8)

Owner review required: yes — a store migration (plugins/stewards/store.ts, new assessments and assessment_divergences tables); decide list entry "A store migration" (bb-plugins/plugins/*/store.ts) is decide: project, and the stewards-plugin entry matches the rest of the diff, so the project steward approves.

Resolved since round 1

  • Blocker 1 (untested swallow path): resolved by bed18b8 — the new test drops the assessments table, rings, and asserts the reconcile still stores the binding's snapshot and logs the stewards: shadow assessment failed warning. I re-ran it with the try/catch removed and it fails, so the swallow is now pinned.

The diff since b85dd13 is that test only (14 lines) plus the PR-body CI correction; no scope or behaviour change, so the lane stands.

Tested

  • Ran: npm test (1576 passed, 34 files) and npx tsc --noEmit in plugins/stewards — both pass at bed18b8.

Looks good.

— from bb thread thr_fh6fwzukit

@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Approved at bed18b8. Nothing to add.

— from bb thread thr_rkn6gtwp8m

@trotterdylan
trotterdylan merged commit b489e10 into main Nov 10, 2026
22 checks passed
@trotterdylan

Copy link
Copy Markdown
Collaborator Author

Merged: owner-reviewed lane, reviewer approved bed18b8, checks green, approved by the project steward.
merge commit b489e10
Note: merging this moves the pin and stages a restart for every developer's server (Restart now, or in the 03:00–05:00 ET quiet window).
dispatched bb-server.yaml on bitcomplete/bb-gate@main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant