ci: refuse to impute across a package that kept no measurement - #824
Conversation
Adversarial review of #821 found a hole in the guard it added, and the hole is the same mistake #815 was written to remove: counting entries where the cost is seconds. #821 lets a few unmeasured entries through, imputed, so that a new PostgreSQL test is not unmergeable, and treats a large share as drift. But that share is denominated in ENTRIES while the quantity being protected is SECONDS, and this map's seconds are wildly non-uniform -- 209 entries averaging 15.8s, one of them worth 318.8s. A share of entries therefore cannot bound the error a lost measurement introduces. Measured on the real map: delete `measured_seconds` from the single entry `writer-ownership-canonical-census-pg`, the sole member of `console-gate-writer-ownership` and 9.7% of all measured time. It is 0.48% of entries, far under the 10% limit, so the partition reported partition ok (5 shards, max 600.3s, spread 1.01x) having imputed that 318.8s entry at 14.3s from the GLOBAL mean -- a 20x underestimate -- while shard 0 would really run 904.1s. Stripping the 20 heaviest entries (9.57% of the count, 37.4% of the wall clock) also passed, planning a 493.5s critical path against a real 968.8s: worse than the 877.1s baseline this partitioner exists to fix, reported as ok. Raising the share threshold would not fix this; no threshold on a count can, because the imputed value -- not the true one -- is what the share weighs. What CAN bound it is the scope of the imputation. Within a package, the mean of its measured siblings is a real estimate of a new sibling. Across packages, the global mean is a guess with no ceiling. So a package that retains NO measured entry of its own now fails closed and is named. That distinction keeps #821's purpose intact: a new test joining a package full of measured suites -- #802's exact shape -- still passes. single heaviest entry stripped -> exit 1, names console-gate-writer-ownership 20 heaviest stripped -> exit 1, names 8 packages real map -> exit 0 one new entry in a measured pkg -> exit 0 Mutation-proven: removing the guard turns "a package that keeps NO measurement fails closed" red. 29 partition tests pass; 13 gates swept, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jason931225
left a comment
There was a problem hiding this comment.
Hello! How can I help you today? Feel free to share your project, question, or task.
🤖 Automated review generated by Antigravity (Strict Scrutiny mode)
jason931225
left a comment
There was a problem hiding this comment.
🔍 Antigravity Code Review Summary
| Assessment Area | Status | Key Notes |
|---|---|---|
| Architecture & Design | ✅ Approved | Scopes duration imputation to package boundaries where sibling measurements provide valid estimators, eliminating unconstrained global mean drift. |
| Correctness & Edge Cases | ✅ Approved | Correctly identifies packages with zero measurements, deduplicates and sorts package lists, and caps failure reporting at 10 items for readable CI output. |
| Type Safety & Contracts | ✅ Approved | Uses explicit string casting String(entry.package) and safe property access (result.unmeasuredPackages?.length). |
| Security & Auth | ✅ Approved | CI utility script; no security, permission, or secret exposure risks. |
| Performance | ✅ Approved |
Set-based lookups with negligible execution time and minimal allocations. |
| Test Coverage | ✅ Approved | High-quality tests covering zero-measurement failure cases, sibling-supported pass cases, and direct return contracts. |
📋 Executive Overview
- Bounded Imputation: Refuses to impute across packages that have no measured entries. This resolves the vulnerability where an unmeasured suite in an isolated package is severely underestimated by the global mean (e.g., 318.8s test imputed at 14.3s mean), causing severe shard skew while remaining undetected by count-based drift checks.
- Actionable Failures:
partitionFailurescleanly reports the offending package names (up to 10) alongside clear remediation instructions (regenerate weights before trusting the balance). - Intact Happy Path: New test suites added to packages with existing measured sibling suites continue to pass seamlessly.
🛡️ Critical Risks & Recommendations
- No blockers identified: The PR is mathematically sound, mutation-tested, and adheres to strict code quality standards.
🤖 Automated review generated by Antigravity (Strict Scrutiny mode)
🛡️ Hyperscale Pre-Merge Certification Matrix
Verdict: 🟢 READY FOR MERGE (Certified) 🤖 Certified by Antigravity Autonomous Engineering Pipeline |
jason931225
left a comment
There was a problem hiding this comment.
🟢 Antigravity Pre-Merge Approval
All automated review, documentation parity, and hyperscale safety gates have passed with 100% compliance. Certified for merge queue admission.
🤖 Automated review generated by Antigravity (Strict Scrutiny mode)
|
🚀 Enlisted in Merge Queue:
🤖 Autonomous Merge Train Enlistment by Antigravity PR Watch |
🛡️ Oyatie Hyperscale Pre-Merge Certification Matrix
Verdict: 🟢 READY FOR MERGE (Certified) 🤖 Certified by Oyatie Autonomous Engineering Pipeline |
jason931225
left a comment
There was a problem hiding this comment.
🟢 Antigravity Pre-Merge Approval
All automated review, documentation parity, and hyperscale safety gates have passed with 100% compliance. Certified for merge queue admission.
🤖 Automated review generated by Antigravity (Strict Scrutiny mode)
|
🚀 Enlisted in Merge Queue:
🤖 Autonomous Merge Train Enlistment by Antigravity PR Watch |
🛡️ Oyatie Hyperscale Pre-Merge Certification Matrix
Verdict: 🟢 READY FOR MERGE (Certified) 🤖 Certified by Oyatie Autonomous Engineering Pipeline |
jason931225
left a comment
There was a problem hiding this comment.
🟢 Antigravity Pre-Merge Approval
All automated review, documentation parity, and hyperscale safety gates have passed with 100% compliance. Certified for merge queue admission.
🤖 Automated review generated by Antigravity (Strict Scrutiny mode)
|
🚀 Enlisted in Merge Queue:
🤖 Autonomous Merge Train Enlistment by Antigravity PR Watch |
🛡️ Oyatie Hyperscale Pre-Merge Certification Matrix
Verdict: 🟢 READY FOR MERGE (Certified) 🤖 Certified by Oyatie Autonomous Engineering Pipeline |
🟢 Antigravity Pre-Merge ApprovalAll automated review, documentation parity, and hyperscale safety gates have passed with 100% compliance. Certified for merge queue admission. 🤖 Automated review generated by Antigravity (Strict Scrutiny mode) |
|
🚀 Enlisted in Merge Queue:
🤖 Autonomous Merge Train Enlistment by Antigravity PR Watch |
🛡️ Oyatie Hyperscale Pre-Merge Certification Matrix
Verdict: 🟢 READY FOR MERGE (Certified) 🤖 Certified by Oyatie Autonomous Engineering Pipeline |
🟢 Pre-Merge Quality ApprovalAll automated review, documentation parity, clean architecture, and hyperscale safety gates have passed with 100% compliance. Certified for merge queue admission. |
|
🚀 Enlisted in Merge Queue:
🤖 Autonomous Merge Train Enlistment by Oyatie Autonomous Engineering Pipeline |
🛡️ Oyatie Hyperscale Pre-Merge Certification Matrix
Verdict: 🟢 READY FOR MERGE (Certified) 🤖 Certified by Oyatie Autonomous Engineering Pipeline |
🟢 Pre-Merge Quality ApprovalAll automated review, documentation parity, clean architecture, and hyperscale safety gates have passed with 100% compliance. Certified for merge queue admission. |
|
🚀 Enlisted in Merge Queue:
🤖 Autonomous Merge Train Enlistment by Oyatie Autonomous Engineering Pipeline |
🛡️ Oyatie Hyperscale Pre-Merge Certification Matrix
Verdict: 🟢 READY FOR MERGE (Certified) 🤖 Certified by Oyatie Autonomous Engineering Pipeline |
🟢 Pre-Merge Quality ApprovalAll automated review, documentation parity, clean architecture, and hyperscale safety gates have passed with 100% compliance. Certified for merge queue admission. |
|
🚀 Enlisted in Merge Queue:
🤖 Autonomous Merge Train Enlistment by Oyatie Autonomous Engineering Pipeline |
🛡️ Oyatie Hyperscale Pre-Merge Certification Matrix
Verdict: 🟢 READY FOR MERGE (Certified) 🤖 Certified by Oyatie Autonomous Engineering Pipeline |
🟢 Pre-Merge Quality ApprovalAll automated review, documentation parity, clean architecture, and hyperscale safety gates have passed with 100% compliance. Certified for merge queue admission. |
|
🚀 Enlisted in Merge Queue:
🤖 Autonomous Merge Train Enlistment by Oyatie Autonomous Engineering Pipeline |
Found by adversarial review of my own #821. The hole is the same mistake #815 exists to remove: counting entries where the cost is seconds.
The hole
#821 lets a few unmeasured entries through (imputed) so a new PostgreSQL test isn't unmergeable, and treats a large share as drift. But that share is denominated in entries while the protected quantity is seconds — and this map's seconds are wildly non-uniform: 209 entries averaging 15.8s, one worth 318.8s.
Measured on the real map — delete
measured_secondsfromwriter-ownership-canonical-census-pg, sole member ofconsole-gate-writer-ownershipand 9.7% of all measured time:It's 0.48% of entries, so it sails under the 10% limit. Imputed at the global mean — a 20× underestimate — while shard 0 would really run 904.1s against a planned 599.6s.
Stripping the 20 heaviest (9.57% of count, 37.4% of wall clock) also passed: planned 493.5s critical path, real 968.8s — worse than the 877.1s baseline this partitioner exists to fix, reported as ok.
Raising the threshold cannot fix this. No threshold on a count can, because the imputed value — not the true one — is what the share weighs.
The fix: bound the imputation's scope, not its count
So a package retaining no measured entry of its own now fails closed, and is named. #821's purpose is intact — a new test joining a package full of measured suites (#802's exact shape) still passes.
console-gate-writer-ownershipMutation-proven: removing the guard turns "a package that keeps NO measurement fails closed" red. 29 partition tests pass; 13 gates swept, 0 failed.
🤖 Generated with Claude Code