fix(public-stats): stop calling a one-instance sample a fleet, and withhold the pooled count at n=2 (#9168) - #9574
Conversation
…thhold the pooled count at n=2 (#9168) `fleetAccuracy` had a per-instance volume floor (MIN_DECIDED) but nothing bounding the NUMBER of instances. At the one registered instance live today it publishes one operator's own outcomes under fleet framing, next to a risk-control guarantee calibrated by that same instance -- which invites a reader to treat one party's self-report as two independent sources. #9228 already fixed the detector half of this issue (gamingFlagsCaught is null, not 0, below GAMING_MIN_ELIGIBLE). This is the framing half, which did not land. WHAT CHANGES - FLEET_FRAMING_MIN_INSTANCES, deliberately defined AS GAMING_MIN_ELIGIBLE: the n at which a median becomes robust to a single bad contributor is the same n at which "this far above the median" becomes satisfiable. Pinning them together means a reader never sees fleet framing next to "the detector could not run". - A `basis` discriminator on the published block: "fleet" or "single_instance_self_report". The numbers are real and stay published at every n -- only the claim about what they are changes. - The pooled `decidedCount` is withheld (null) at exactly 1 < instanceCount < floor. The count is a plain SUM, and this deployment's own volume is already public via byProject, so at n=2 a reader recovers the OTHER instance's decision volume by subtraction. For a hosted tenant that volume is a business metric -- how many PRs they ship, how many get closed -- and not ours to publish. At n=1 there is nothing to subtract; at n >= floor the sum no longer isolates anyone. RATES are safe at every n, since a proportion carries no volume, so only the count is withheld. - The median-robustness comment in analytics.ts claimed robustness unconditionally. Corrected to state the n it actually requires, since that claim was the reason the median was chosen and it was doing no work at n=1. - The DB-failure fallback fails closed (fleetFramingEligible: false) with the rest of that path -- if the fleet tables cannot be read we certainly cannot claim a fleet. The OpenAPI schema is a hand-maintained zod mirror, so `basis` and the now-nullable `decidedCount` are added there too, and apps/loopover-ui/public/openapi.json regenerated -- otherwise the spec silently drifts from the response it documents. NOT DECIDED HERE: whether a hosted tenant's outcomes should enter the public aggregate at all, and if so opt-in or opt-out. That is a product/privacy call, not a code one. This change is safe under either answer -- it withholds the leaky figure by default rather than presuming consent -- and the issue records the question. Tests: the outlier and gaming thresholds had never been exercised against multi-instance fixtures, which #9168 called out. Now covered at n=1, n=2, n=3, with the eligible-vs-registered distinction (volume and registration both gate the floor) and the fail-closed path. 0 uncovered changed lines or branches.
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-28 11:05:02 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Decision record
Visual preview
Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy. Scroll preview
A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 29627ee | Commit Preview URL Branch Preview URL |
Jul 28 2026, 10:40 AM |
Bundle ReportChanges will increase total bundle size by 349 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9574 +/- ##
==========================================
- Coverage 89.62% 88.73% -0.90%
==========================================
Files 868 868
Lines 110873 110876 +3
Branches 26360 26362 +2
==========================================
- Hits 99371 98383 -988
- Misses 10237 11519 +1282
+ Partials 1265 974 -291
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…d shape #9565 added Two rebase follow-ups after #9565 and #9574 landed. 1. scripts/actionlint.ts gets its `.ts` extension back. This PR had removed it to satisfy check-import-specifiers, which broke the script outright -- it runs under `node --experimental-strip-types`, whose ESM resolver does no extension resolution, so the process dies at startup with ERR_MODULE_NOT_FOUND. #9565 independently reached the same conclusion and added TYPE_STRIPPED_ENTRYPOINTS to the checker for exactly this file, so the extension is now permitted where it is required. Verified by running `npm run actionlint`, which fails before this change and passes after. #9565's version of the checker is taken wholesale over this PR's: it solves the same two problems (that entrypoint set, plus allowlisting check-dead-source-files-script.test.ts for its string fixtures), and re-litigating a file main just rewrote buys nothing. 2. src/mcp/server.ts's `loginRepoPullShape` is removed -- dead on arrival in #9565, and the first thing the newly-enabled noUnusedLocals caught on main. Which is the point of this PR: dead code now surfaces at the commit that introduces it rather than at the next audit. The engine bump lands at 3.16.1 (main released 3.16.0 while this was open). It is required by check-engine-parity: this PR removes two dead TYPE-only imports from packages/loopover-engine/src/advisory/gate-advisory.ts, and the parity contract holds that file in lockstep with its host twin src/rules/advisory.ts. There is no matching host edit to make -- the engine copy is a deliberately slimmed re-implementation (#4881) omitting the functions that use those types -- so the version bump is the sanctioned way to record a one-sided change. No behaviour change: type-only imports are erased at compile time. packages/loopover-miner/expected-engine.version moves with it, as its own check requires.
…d shape #9565 added Two rebase follow-ups after #9565 and #9574 landed. 1. scripts/actionlint.ts gets its `.ts` extension back. This PR had removed it to satisfy check-import-specifiers, which broke the script outright -- it runs under `node --experimental-strip-types`, whose ESM resolver does no extension resolution, so the process dies at startup with ERR_MODULE_NOT_FOUND. #9565 independently reached the same conclusion and added TYPE_STRIPPED_ENTRYPOINTS to the checker for exactly this file, so the extension is now permitted where it is required. Verified by running `npm run actionlint`, which fails before this change and passes after. #9565's version of the checker is taken wholesale over this PR's: it solves the same two problems (that entrypoint set, plus allowlisting check-dead-source-files-script.test.ts for its string fixtures), and re-litigating a file main just rewrote buys nothing. 2. src/mcp/server.ts's `loginRepoPullShape` is removed -- dead on arrival in #9565, and the first thing the newly-enabled noUnusedLocals caught on main. Which is the point of this PR: dead code now surfaces at the commit that introduces it rather than at the next audit. The engine bump lands at 3.16.1 (main released 3.16.0 while this was open). It is required by check-engine-parity: this PR removes two dead TYPE-only imports from packages/loopover-engine/src/advisory/gate-advisory.ts, and the parity contract holds that file in lockstep with its host twin src/rules/advisory.ts. There is no matching host edit to make -- the engine copy is a deliberately slimmed re-implementation (#4881) omitting the functions that use those types -- so the version bump is the sanctioned way to record a one-sided change. No behaviour change: type-only imports are erased at compile time. packages/loopover-miner/expected-engine.version moves with it, as its own check requires.
…#9553, #9570, #9571, #9572) (#9573) * build(typescript): enable noUnusedLocals/noUnusedParameters repo-wide (#9553) Dead code is the substrate every drift bug in the 2026-07-27 audit grew on: a stale import or an orphaned constant reads exactly like a live wire, so the next person greps, finds it, and reasons about a code path that no longer runs. Enabling the flags made the compiler enumerate all 515 instances. Every one in src/** and packages/** was traced to its replacement before deletion -- all 82 were genuine supersession leftovers, no behaviour bug hiding among them -- but the triage turned up three real problems that were invisible under the noise: 1. src/queue/processors.ts -- sweepRepoBacklogConvergence accepted `requestedBy` ("schedule" | "api" | "test") and dropped it. Every sibling sweep stamps it into recordAuditEvent's metadata; both agent.sweep.backlog_convergence events here omitted it, so those records could not be attributed to a schedule vs a manual API trigger. Now wired into both. (Note the mechanical fix would have been to rename it `_requestedBy`, which cements the gap instead of closing it.) 2. test/unit/openapi.test.ts -- the #9302 REST<->MCP parity guard asserted against src/mcp/server.ts's gatePrecisionOutputSchema and maintainerMeasurementReportOutputSchema, which the tools stopped registering when #9518 moved their outputs to @loopover/contract. The shapes are still identical, so nothing had drifted YET -- but the guard was watching objects no runtime reads and would not have caught a future contract change. Re-anchored onto GetGatePrecisionOutput.shape / GetOutcomeCalibrationOutput.shape, which is what the tools actually register, and what that file's own header comment already claimed it did. 3. src/github/resolve-command.ts was reachable only from its own test, because src/review/review-memory-wire.ts carried a SECOND byte-identical copy of normalizeResolveFindingRef (regex included) and that copy was the one production used. Two independent implementations of the same public-safety validation, free to drift. Deduped onto the original via re-export; dead-source-files:check now passes on a file it was about to start failing on. Also corrects packages/loopover-engine/src/scoring/preview.ts's header, which claimed a ReDoS guarantee via a hasUnsafeWildcardCount import that had been dead since 625e236 deduped its label matching onto label-match.ts. The guarantee is real and unchanged; it now arrives through labelMatchesPattern, and the comment says so. Pre-existing import-specifier violations fixed in the same pass, since the tree has to be green for the flags to mean anything: - scripts/actionlint.ts imported a `.ts` specifier (TS5097) - test/unit/contract-registry.test.ts had three `.js` specifiers in a Bundler zone - check-dead-source-files-script.test.ts tripped check-import-specifiers on its own string FIXTURES, the same self-referential false positive that checker's ALLOWED_FILENAMES already documents for its own test Mechanics: unused parameters are renamed with a leading underscore, never deleted -- they are positional, so removing one silently re-binds every later argument. Everything else was removed by its real TypeScript AST node span (a regex pass was tried first and mis-bounded declarations badly enough to produce unparseable files). Full suite green: 23,894 passed, 0 failed. tsc clean with the flags on. * chore(engine): bump to 3.15.4 for the dead type-import removal in gate-advisory.ts check-engine-parity holds the two hand-duplicated gate-decision twins (packages/loopover-engine/src/advisory/gate-advisory.ts and src/rules/advisory.ts) in lockstep: touching one without the other requires an engine version bump. That is the mechanism, and it is doing its job here. the engine twin. They are genuinely dead there and NOT in the host: the engine copy is a deliberately slimmed re-implementation (#4881) that omits buildIssueAdvisory / addIssueFindings / collisionClustersForPull, which are what use those types on the host side. So there is no matching host edit to make -- the asymmetry is correct, and the version bump is the sanctioned way to record it. No behaviour change: type-only imports are erased at compile time. The bump exists so the parity contract stays enforceable, not because the gate decides anything differently. packages/loopover-miner/expected-engine.version moves in lockstep, as its own check requires. * fix(scripts,mcp): restore actionlint's `.ts` specifier and drop a dead shape #9565 added Two rebase follow-ups after #9565 and #9574 landed. 1. scripts/actionlint.ts gets its `.ts` extension back. This PR had removed it to satisfy check-import-specifiers, which broke the script outright -- it runs under `node --experimental-strip-types`, whose ESM resolver does no extension resolution, so the process dies at startup with ERR_MODULE_NOT_FOUND. #9565 independently reached the same conclusion and added TYPE_STRIPPED_ENTRYPOINTS to the checker for exactly this file, so the extension is now permitted where it is required. Verified by running `npm run actionlint`, which fails before this change and passes after. #9565's version of the checker is taken wholesale over this PR's: it solves the same two problems (that entrypoint set, plus allowlisting check-dead-source-files-script.test.ts for its string fixtures), and re-litigating a file main just rewrote buys nothing. 2. src/mcp/server.ts's `loginRepoPullShape` is removed -- dead on arrival in #9565, and the first thing the newly-enabled noUnusedLocals caught on main. Which is the point of this PR: dead code now surfaces at the commit that introduces it rather than at the next audit. The engine bump lands at 3.16.1 (main released 3.16.0 while this was open). It is required by check-engine-parity: this PR removes two dead TYPE-only imports from packages/loopover-engine/src/advisory/gate-advisory.ts, and the parity contract holds that file in lockstep with its host twin src/rules/advisory.ts. There is no matching host edit to make -- the engine copy is a deliberately slimmed re-implementation (#4881) omitting the functions that use those types -- so the version bump is the sanctioned way to record a one-sided change. No behaviour change: type-only imports are erased at compile time. packages/loopover-miner/expected-engine.version moves with it, as its own check requires. * chore(release): sync .release-please-manifest.json to the 3.16.1 engine bump The manifest is a generated artifact that must move with any package.json version, and release-manifest:sync:check fails CI when it drifts. Regenerated with the repo's own `npm run release-manifest:sync` rather than hand-edited. * chore: prune the dead symbols the new flags caught in newly-merged code Rebase onto main after #9579 and #9580 landed. The newly-enabled noUnusedLocals immediately flagged twelve dead symbols in code merged since this PR opened -- including two I left in #9580 myself: - src/queue/processors.ts: PrCommandPrologueOutcome imported but unused (the adapter's annotated return type was dropped in favour of inference), plus eight unused bindings in the prologue destructures -- handlers that do not need `pr`, `settings`, `authorization` or `command` were still pulling them out. - src/queue/pr-command-prologue.ts: LoopOverMentionCommandName, superseded by LoopOverActionCommandName once the spec narrowed to action verbs. - src/mcp/dispatch-telemetry-sink.ts: an unused McpToolCallTelemetry import from #9579. Which is the point of the PR: the flags catch dead code at the commit that introduces it rather than at the next audit. Zero behaviour change -- every removal is a binding or an import TypeScript proved unreferenced, and the suite passes 24,014 tests. The rebase conflict itself was in processors.ts's import block: main added the pr-command-prologue import on the same lines this PR removed the unused runRetentionPrune one. Both intents kept.
…can deploy Cloudflare Workers Builds has failed on every recent commit, which is the deploy path for loopover-ui — no GitHub Actions workflow runs `wrangler deploy` (ui-deploy.yml only validates). Production has consequently been serving stale code: #9574's `fleetAccuracy.basis` and #9594's anchor-payload auth exemption both landed on main and neither is live. The build command is `npm run build:cloudflare` -> `npm ci && npm run ui:build`. `ui:build` runs `turbo run build --filter=@loopover/engine`, then `ui:openapi`, whose `scripts/write-ui-openapi.ts` reaches `src/openapi/schemas.ts` and imports `@loopover/contract/dist/public-api.js`. Nothing in that chain builds `@loopover/contract` — `@loopover/engine` does not depend on it — so on a clean checkout the import resolves to a directory that was never emitted: Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/opt/buildhome/repo/node_modules/@loopover/contract/dist/public-api.js' imported from /opt/buildhome/repo/src/openapi/schemas.ts It passes in `test:ci` only by accident of ordering: `npm run typecheck` runs first, and the `//#typecheck` turbo task already carries an explicit `@loopover/contract#build` edge — added for exactly this failure mode, and documented in turbo.json as "the root package.json dependency does NOT create a build edge for a root task". Cloudflare runs `ui:build` on its own, with no typecheck ahead of it, so the incidental ordering never applies. Adding the same edge to `ui:build`'s own turbo invocation, which is the level that actually owns the dependency. Verified against a genuinely clean tree (package dists and .tsbuildinfo files removed, TURBO_FORCE=1 to defeat the shared worktree cache — the first two attempts at reproducing this were masked by a stale tsbuildinfo and then by a turbo cache replay): without this change `ui:build` exits 1 on the error above; with it, exit 0 and both dist artifacts are emitted.
…checkout can deploy Cloudflare Workers Builds is the deploy path for BOTH workers — no GitHub Actions workflow runs `wrangler deploy` (ui-deploy.yml only validates) — and both have failed on every recent commit. Production has consequently been serving stale code: #9574's `fleetAccuracy.basis` and #9594's anchor-payload auth exemption both landed on main and neither is live. One root cause, two scripts: nothing builds `@loopover/contract` before something needs its `dist/`. `@loopover/engine` does not depend on it, so building engine alone leaves it unemitted. loopover-ui — `npm run ui:build` runs `turbo run build --filter=@loopover/engine`, then `ui:openapi`, whose `scripts/write-ui-openapi.ts` reaches `src/openapi/schemas.ts` and imports `@loopover/contract/dist/public-api.js`: Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/opt/buildhome/repo/node_modules/@loopover/contract/dist/public-api.js' loopover-api — `npm run deploy:api` builds only `@loopover/engine`, then `wrangler deploy` bundles `src/index.ts`, which reaches `@loopover/contract` from src/mcp/*, src/openapi/schemas.ts and src/orb/broker-client.ts: ✘ [ERROR] Build failed with 8 errors: ✘ [ERROR] Could not resolve "@loopover/contract" Both pass in `test:ci` only by accident of ordering: `npm run typecheck` runs first, and the `//#typecheck` turbo task already carries an explicit `@loopover/contract#build` edge — added for exactly this failure mode, and documented in turbo.json as "the root package.json dependency does NOT create a build edge for a root task". Neither deploy command runs a typecheck ahead of itself, so the incidental ordering never applies. Adding the same edge to each deploy path's own turbo invocation, which is the level that actually owns the dependency. `deploy:api` also moves from a bare `npm --workspace ... run build` to turbo so both paths express it identically. Verified against a genuinely clean tree (package dists and .tsbuildinfo files removed, TURBO_FORCE=1 to defeat the shared worktree cache — the first two reproduction attempts were masked by a stale tsbuildinfo and then by a turbo cache replay). ui:build: exit 1 -> exit 0. wrangler deploy --dry-run: 8 unresolved "@loopover/contract" imports -> 0, exit 0.
…checkout can deploy (#9732) Cloudflare Workers Builds is the deploy path for BOTH workers — no GitHub Actions workflow runs `wrangler deploy` (ui-deploy.yml only validates) — and both have failed on every recent commit. Production has consequently been serving stale code: #9574's `fleetAccuracy.basis` and #9594's anchor-payload auth exemption both landed on main and neither is live. One root cause, two scripts: nothing builds `@loopover/contract` before something needs its `dist/`. `@loopover/engine` does not depend on it, so building engine alone leaves it unemitted. loopover-ui — `npm run ui:build` runs `turbo run build --filter=@loopover/engine`, then `ui:openapi`, whose `scripts/write-ui-openapi.ts` reaches `src/openapi/schemas.ts` and imports `@loopover/contract/dist/public-api.js`: Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/opt/buildhome/repo/node_modules/@loopover/contract/dist/public-api.js' loopover-api — `npm run deploy:api` builds only `@loopover/engine`, then `wrangler deploy` bundles `src/index.ts`, which reaches `@loopover/contract` from src/mcp/*, src/openapi/schemas.ts and src/orb/broker-client.ts: ✘ [ERROR] Build failed with 8 errors: ✘ [ERROR] Could not resolve "@loopover/contract" Both pass in `test:ci` only by accident of ordering: `npm run typecheck` runs first, and the `//#typecheck` turbo task already carries an explicit `@loopover/contract#build` edge — added for exactly this failure mode, and documented in turbo.json as "the root package.json dependency does NOT create a build edge for a root task". Neither deploy command runs a typecheck ahead of itself, so the incidental ordering never applies. Adding the same edge to each deploy path's own turbo invocation, which is the level that actually owns the dependency. `deploy:api` also moves from a bare `npm --workspace ... run build` to turbo so both paths express it identically. Verified against a genuinely clean tree (package dists and .tsbuildinfo files removed, TURBO_FORCE=1 to defeat the shared worktree cache — the first two reproduction attempts were masked by a stale tsbuildinfo and then by a turbo cache replay). ui:build: exit 1 -> exit 0. wrangler deploy --dry-run: 8 unresolved "@loopover/contract" imports -> 0, exit 0.


Closes #9168
Summary
fleetAccuracyhad a per-instance volume floor (MIN_DECIDED) but nothing bounding the number of instances. At the one registered instance live today, it publishes one operator's own outcomes under fleet framing — next to a risk-control guarantee calibrated by that same instance, which invites a reader to treat one party's self-report as two independent sources.#9228 already landed the detector half of #9168 (
gamingFlagsCaughtisnull, not0, belowGAMING_MIN_ELIGIBLE). This is the framing half, which did not.What changes
FLEET_FRAMING_MIN_INSTANCES, defined asGAMING_MIN_ELIGIBLE. The n at which a median becomes robust to a single bad contributor is the same n at which "this far above the median" becomes satisfiable. Pinning them together means a reader never sees fleet framing sitting next to "the detector could not run" — and neither floor can be moved without the other.A
basisdiscriminator on the published block:"fleet"or"single_instance_self_report". The numbers are real and stay published at every n; only the claim about what they are changes.The pooled
decidedCountis withheld at exactly1 < instanceCount < floor. The count is a plain sum, and this deployment's own volume is already public viabyProject— so at n=2 a reader recovers the other instance's decision volume by subtraction. For a hosted tenant that volume is a business metric (how many PRs they ship, how many get closed), not ours to publish.The median-robustness comment was wrong.
analytics.tsjustified the median as "robust to a single bad contributor" unconditionally. At n=1 the median is that instance's value; at n=2 it is the mean of two, which a single bad contributor moves by half its error. That claim was the reason the median was chosen, and it was doing no work at the n we actually run at. Corrected to state the n it requires.The DB-failure fallback fails closed (
fleetFramingEligible: false) with the rest of that path — if the fleet tables cannot be read, we certainly cannot claim a fleet.OpenAPI
The spec is a hand-maintained zod mirror, so
basisand the now-nullabledecidedCountare added there too andapps/loopover-ui/public/openapi.jsonregenerated — otherwise the spec silently drifts from the response it documents.Deliberately not decided here
Whether a hosted tenant's outcomes enter the public aggregate at all, and if so opt-in or opt-out. That is a product/privacy call, not a code one, and #9168 asks for it to be recorded before the first tenant lands.
This change is safe under either answer: it withholds the leaky figure by default rather than presuming consent. If the answer turns out to be "tenants opt in and pooled counts are fine", the floor relaxes; if it is "never", nothing further is needed.
Testing
#9168 called out that the outlier and gaming thresholds "have never been exercised against real multi-instance data". They now are — n=1, n=2, and n=3 fixtures, plus:
"fleet"label always arrives with a realgamingFlagsCaught(0, not null), since the two share a floorsrc/orb/analytics.tsandsrc/review/public-stats.ts: 0 uncovered changed lines, 0 uncovered branches.Note on issue linkage
One issue rather than the usual bundle: the sibling ORB issues in this range are either now rescoped after verification (#9136 — see my comment there; roughly half of it was already fixed and one section is factually stale) or must land alone by their own requirement (#9541). #9068 and #8835, which #9168 cross-references, are already closed.