The tenancy comment said 23 of 42; the pins said 24 and 43 - #125
Conversation
`registry.ts`'s doc comment is what a developer reads to learn what the tenant boundary actually covers. It said "only 23 of 42 models carry institutionId". The pins in `registry.test.ts` said 24 and 43, and they were right — they are compared against the real schema, so they cannot drift. Two changes on the same day each INCREMENTED a number that was already one behind rather than measuring it, and the suite stayed green through both, because nothing compared the sentence to the pins. That is the gap: the ledger's counts are cross-checked by constitution-completeness-compiler.test.ts, and the pins are checked against the schema, but the human-readable claim in the source file sits between them unguarded. It is also the one a reader is most likely to trust, since it is right next to the code it describes. Corrects the numbers and adds the missing comparison. The new test reads registry.ts, extracts the two figures from that sentence, and requires them to equal TENANT_SCOPED.length and schemaModels.length. It also asserts the three buckets still sum to the model count, so the pair cannot agree with each other while both are wrong. Two negative controls, each run after committing: prose reverted to "23 of 42" -> 1 failed / 11 passed TENANT_SCOPED pin changed to 23 -> 1 failed / 11 passed Restored: 12 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
satvikOS has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe tenancy registry documentation updates the model counts to 24 of 43. A new test validates these counts against the registry and parsed schema, and confirms complete classification coverage. ChangesTenancy registry consistency
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change corrects the tenancy counts shown to developers and adds a guard against future documentation drift; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
main took #107 (the seat meter, ADR-0017/0018), #124 (ElastiCache removed), #125 (the tenancy prose corrected against its own pins) and #126. Six conflicts, and every one of them is a counter or a registry that exists to make exactly this loud rather than silent: - schema.prisma both sides append disjoint models; kept both. - capabilities.ts exception.resolve/waive and billing.viewMeter; kept both. - registry.test.ts the four pinned counts. MEASURED against schema.prisma, not incremented: this branch was written against 41 models / 22 tenant-scoped and main had moved to 43/24, so either side's number carried forward alone would have been wrong by two. Now 44 models, 25 TENANT_SCOPED / 5 PLATFORM_GLOBAL / 14 UNENFORCEABLE, which sums to 44. - registry.ts the doc comment sentence #125 added a test for. 25 of 44, with a dated rationale. - docs/decisions ADR-0015 LANDS here, so its reservation row is DELETED. 0016 stays reserved. 9 of 16 are Proposed. - the ledger counts-provenance and the SIMON-030-010 row, reconciled to the same measured numbers. And one thing git reported no conflict for, because the directory names differ: the migration 20260820140000_exception_register collided with main's 20260820140000_idempotent_accounting_intake. Two migrations sharing a timestamp is a broken deploy rather than a red merge, so it is bumped to 20260821093000_exception_register — after everything on main.
main took #125 (the tenancy doc comment corrected against its own pins) and #126 (the DKIM token check). This branch had already merged main at 2d4469a for #107 and #124. One conflict, and one thing that was NOT a conflict and mattered more: - registry.ts the doc-comment sentence #125 added a test for. Kept this branch's 26 of 45; main's 24 of 43 does not know about OnboardingProposal or OnboardingProposalEvent. - registry.test.ts AUTO-MERGED. The four pinned counts are the assertion that actually guards the tenancy boundary and git resolved them silently from one side. They were re-derived from schema.prisma with the test's own parser rather than trusted: 45 models, 26 carrying institutionId, and 26 + 5 + 14 = 45. They were already correct, but only measuring could say so. Migration timestamps checked against main: this branch's two (20260821090000_ose_initiated_onboarding_proposals and 20260821140000_decline_states_a_reason) collide with nothing. The one duplicate in the directory, 20260820120000, is main's own pair and predates this branch.
Resolved five conflicts, all of them two changes adding beside each other
rather than disagreeing:
* prisma/schema.prisma — Institution gains all three back-relations;
TenantConfigPack and the seat-metering block are both kept.
* tenancy/registry.ts + registry.test.ts — the pins are MEASURED against
the merged schema, not incremented. grep -c '^model ' = 44, and the
three buckets are 25 / 5 / 14, which sums to 44. #125's new
prose-agreement case is kept and the doc sentence updated with it.
* docs/decisions/README.md — this change takes ADR-0021 per the
allocation on PR #106, and declares 0020 as a reservation because
#112 has not merged yet.
* docs/implementation/global-engine-execution-ledger.md — same measured
counts, with this change's step appended rather than restated.
ADR-0015-tenant-configuration-packs.md is renamed to ADR-0021 and every
cross-reference moved with it: the ADR body, the index table, the backlog,
.env.example, publish-tenant-packs.yml and four source comments.
registry.ts's doc comment is what a developer reads to learn what the tenant boundary covers. Onmainit says:The pins in
registry.test.tssay 24 and 43 — and they are right. They are compared against the real schema, so they cannot drift.How it drifted
Two changes on the same day each incremented a number that was already one behind, rather than measuring it. The suite stayed green through both, because nothing compared the sentence to the pins.
24 + 5 + 14 = 43✓ — the pins are internally consistent. Only the prose was wrong.The gap this closes
constitution-completeness-compiler.test.tsregistry.ts's own sentenceIt sits between two guarded things, unguarded — and it is the claim a reader is most likely to trust, because it is immediately above the code it describes.
The fix
Corrects the numbers, and adds the missing comparison: the new test reads
registry.ts, extracts both figures from that sentence, and requires them to equalTENANT_SCOPED.lengthandschemaModels.length.It also asserts the three buckets still sum to the model count — otherwise the pair could agree with each other while both are wrong.
Negative controls, each run after committing
TENANT_SCOPEDpin changed to 23Note for whoever lands a model next
Three models arrived in one day (
RestrictedRegistrySeal,SeatMeterEvent,OnboardingProposal), each numbered against a different starting count. PR #118's rebase measures 44 / 25 once its model lands. This guard means that sentence now has to be updated with them — which is the point.Summary by CodeRabbit
Documentation
Tests