Skip to content

The tenancy comment said 23 of 42; the pins said 24 and 43 - #125

Merged
satvikOS merged 1 commit into
mainfrom
fix/registry-prose-drift
Aug 21, 2026
Merged

The tenancy comment said 23 of 42; the pins said 24 and 43#125
satvikOS merged 1 commit into
mainfrom
fix/registry-prose-drift

Conversation

@satvikOS

@satvikOS satvikOS commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

registry.ts's doc comment is what a developer reads to learn what the tenant boundary covers. On main it says:

only 23 of 42 models carry institutionId

The pins in registry.test.ts say 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

guarded by
the ledger's counts constitution-completeness-compiler.test.ts
the pins compared against the real schema
registry.ts's own sentence nothing

It 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 equal TENANT_SCOPED.length and schemaModels.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

control result
prose reverted to "23 of 42" 1 failed / 11 passed
TENANT_SCOPED pin changed to 23 1 failed / 11 passed
restored 12 passed

Note 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

    • Updated tenancy registry documentation with current model counts.
    • Clarified that documented counts are validated against registry and schema checks.
  • Tests

    • Added consistency checks to ensure documented counts, registered models, schema models, and classification groups remain aligned.

`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>

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

satvikOS has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: de2122fb-8b5f-4f84-a2c7-b721d5bdee53

📥 Commits

Reviewing files that changed from the base of the PR and between 7279b9e and 1afca00.

📒 Files selected for processing (2)
  • apps/web/src/lib/tenancy/registry.test.ts
  • apps/web/src/lib/tenancy/registry.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The 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.

Changes

Tenancy registry consistency

Layer / File(s) Summary
Registry count documentation and validation
apps/web/src/lib/tenancy/registry.ts, apps/web/src/lib/tenancy/registry.test.ts
The registry documents 24 of 43 models with institutionId. The test validates the documented counts against registered and schema-derived models and verifies that all classification buckets cover every schema model.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 1afca

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)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the outdated tenancy counts and matches the main correction in the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/registry-prose-drift

Comment @coderabbitai help to get the list of available commands.

@satvikOS
satvikOS merged commit 69f6f75 into main Aug 21, 2026
5 checks passed
@satvikOS
satvikOS deleted the fix/registry-prose-drift branch August 21, 2026 06:26
satvikOS pushed a commit that referenced this pull request Aug 21, 2026
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.
satvikOS pushed a commit that referenced this pull request Aug 21, 2026
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.
satvikOS pushed a commit that referenced this pull request Aug 21, 2026
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.
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.

2 participants