Skip to content

test(lint): index the access-matrix rows once in the exhaustive fold case - #22358

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-22346-access-matrix-linear-lookup
Oct 8, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-22346-access-matrix-linear-lookup

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Part of #22346

Clause-②: no

What changed

One test case, test-only: packages/lint/src/build-access-matrix.test.ts, "exhaustively matches the fold over every declared bit combination". Its per-entry m.entries.find(...) lookup is replaced by one Map, built once from m.entries and keyed by permissionSet. The first match wins, so every lookup returns the same row find returned. The 3^8 enumeration, the cell count and all four assertions are unchanged. build-access-matrix.ts is untouched, no timeout is raised, and nothing is skipped.

Why Part of: the card's generator closes the card when an hourly run is green. Its second cause, the shard-timing drift, belongs to #22014, which remains open. This PR removes only the lint case's half.

Measurements

All wall times were taken under scripts/pm/os-verify-lock.sh in a shared container. The lock excludes other locked runs, not unlocked sibling work, so the ratios carry more weight than the absolute numbers.

H1: the lookup is quadratic (confirmed). A counter in the find callback, written the same way as the case, read 21,526,641 calls per run. That is 6,561 × 6,562 / 2, and each call rebuilds the ps_NNNN name.

H2: the lookup dominates (confirmed). A scratch script (not committed) timed each step of the case three times at 54c3ce10c:

step iter 1 iter 2 iter 3
enumerate the 3^8 entries 4.5 ms 1.0 ms 0.9 ms
map entries to permission sets 1.6 ms 1.9 ms 0.6 ms
buildAccessMatrix 25.0 ms 14.7 ms 3.1 ms
lookup, per-entry find (as on main) 950.8 ms 779.8 ms 845.7 ms
lookup, Map built once 2.7 ms 1.3 ms 1.7 ms
fold comparison loop 5.6 ms 1.7 ms 1.7 ms

No other step in the case is quadratic. buildAccessMatrix, its sort included, costs too little to report.

Case wall time, as vitest reports it per test. Command: pnpm --filter @objectstack/lint exec vitest run --maxWorkers=2 --reporter=verbose src/build-access-matrix.test.ts, three runs per tree:

tree run 1 run 2 run 3
before, 54c3ce10c 700 ms 613 ms 751 ms
after, b447bfa07 46 ms 44 ms 47 ms

That is about 15x faster. CI crossed the 5 s default only on a loaded shard (the triage measured a 473 s import phase there). At this container's speed, the case now runs at about 1/100 of the limit.

Ablation. scripts/ablation-replace.mjs put the per-entry find back in place of the Map lookup three times, each in wrap mode. Each time the anchor count went 1 → 0, the replacement count went 0 → 1, and the blob went 84866b0a6a8f → 40761cc2c49a.

leg run 1 run 2 run 3
find put back (mutation) 834 ms 803 ms 696 ms

All 8 cases passed under the mutation, which is expected: the timing is the pin, and no assertion was added. On every restore leg, the blob after restore equalled the blob at HEAD (84866b0a6a8f), and git diff HEAD was empty.

The assertions are unchanged

The case has 4 expect( calls before and after the change. Their matcher text is identical (compared as text with cmp):

expect(entries.length).toBe(3 ** 8)
expect(m.entries.length).toBe(entries.length)
expect(cells).toBe(3 ** 8 * 4)
expect(mismatches).toEqual([])

Across the whole file there are 28 expect( lines before and after, byte-identical as a sorted set, and 8 it( cases before and after. The full diff of the case:

     expect(m.entries.length).toBe(entries.length);
+    // Index the rows by permission set ONCE. A `find` per entry is quadratic —
+    // 3^8 entries make ~21.5M callbacks, each rebuilding the name — which ran
+    // this case past vitest's 5 s default on a loaded CI shard. First match
+    // wins, so every lookup returns the row `find` returned.
+    const rowByPermissionSet = new Map TYPE_ARGS ();
+    for (const e of m.entries) {
+      if (!rowByPermissionSet.has(e.permissionSet)) rowByPermissionSet.set(e.permissionSet, e);
+    }
     const mismatches: string[] = [];
     let cells = 0;
     for (let i = 0; i LESS_THAN entries.length; i++) {
-      const row = m.entries.find((e) => e.permissionSet === `ps_${String(i).padStart(4, '0')}`)!;
+      const row = rowByPermissionSet.get(`ps_${String(i).padStart(4, '0')}`)!;

Two placeholders stand in for angle-bracket text, which GitHub's body sanitizer removes. TYPE_ARGS is the Map's two type arguments: string, and the entry type written as (typeof m.entries)[number]. LESS_THAN is the less-than operator. The committed lines are in the file.

Verification (at b447bfa07)

  • pnpm --filter @objectstack/lint exec vitest run --maxWorkers=2: 127 files passed; 5838 tests passed and 5 were skipped, out of 5843.
  • pnpm --filter @objectstack/lint typecheck: exit 0.
    • The edited file is in tsconfig.test.json's program (--listFiles: 1 hit). The build tsconfig.json excludes it (0 hits).
    • check:test-typecheck passes, and this file is not on the debt ledger, so it compiles with zero errors.
  • ESLint, narrowed to the one changed file: eslint --no-inline-config --format json packages/lint/src/build-access-matrix.test.ts reports 1 file, 0 errors, 0 warnings.
    • Population: the diff's only path. --print-config shows that ESLint lints it, with 5 rules.
    • Invariance: the config enables no type-aware linting for this file (parserOptions.project and projectService are both null), and the diff edits no config. So no untouched file's verdict can change.
    • The repo-wide pnpm lint belongs to CI.
  • Gates: node scripts/pm/dispatch-gates.mjs --commands (no paths) at b447bfa07 derives the same 54 commands as the list taken at dispatch.
    • 52 ran green on the first pass.
    • check:lean-entry-closure first refused with PREREQUISITE NOT MET. It is green after building the @objectstack/objectql closure.
    • check:dual-build-cjs-loads refused with PREREQUISITE NOT MET because it needs every package built. It is NOT MEASURED locally and is left to CI.
    • --ran verdict: "54 derived famil(ies) accounted for — 53 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3)".

Changeset

No changeset; skip-changeset is applied instead.

  • The diff is test-only. Under AGENTS.md, the label is for a diff that publishes nothing from any released package.
  • Measured after pnpm --filter @objectstack/lint build: the package's files are dist, README.md and CHANGELOG.md. The new symbol rowByPermissionSet and the case title have 0 hits there; the positive control buildAccessMatrix has 6.
  • check-empty-changeset.mjs refuses an empty-frontmatter changeset, so the label is the declared opt-out.

Acceptance notes


Generated by Claude Code

…case

The "exhaustively matches the fold over every declared bit combination"
case looked each of its 3^8 = 6,561 rows up with `m.entries.find(...)`,
rebuilding the `ps_NNNN` name inside the callback: 21,526,641 callback
calls, quadratic in the entry count. It ran 0.6-0.75 s on a quiet box and
crossed vitest's 5 s default on a loaded CI shard.

Index the rows by permission set once in a Map (first match wins, so each
lookup returns the row `find` returned). The enumeration, the cell count
and all four assertions are unchanged.

Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet objectstack-fleet Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Oct 8, 2026
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

What this run could not see

Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 1cb0edb82dcc7fb67a5eec1157c23900e3915924 → packageMentionDocs.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 8, 2026 22:37
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 8, 2026 22:37
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 8, 2026
Merged via the queue into main with commit b460153 Oct 8, 2026
40 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-22346-access-matrix-linear-lookup branch October 8, 2026 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/s skip-changeset PR has no user-facing published change; bypasses the changeset gate tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants