Skip to content

SCALE-SEAM (95) — element state: two read/write pairs, and why the matrix loses - #412

Merged
ibuilder merged 1 commit into
mainfrom
claude/codebase-audit-roadmap-45656d
Sep 4, 2026
Merged

SCALE-SEAM (95) — element state: two read/write pairs, and why the matrix loses#412
ibuilder merged 1 commit into
mainfrom
claude/codebase-audit-roadmap-45656d

Conversation

@ibuilder

@ibuilderibuilder commented Sep 4, 2026

Copy link
Copy Markdown
Owner

What & why

apps/web/src/api/client.ts goes 727 → 711 (76 methods above the STAYING banner, 4 below).
Four methods move to apps/web/src/api/model.ts — the lodSummary/setLod and phasing/setPhase
read/write pairs — because they answer one question: what state are the model's elements in, and
set it?
Continues SCALE-SEAM in docs/roadmap.md.

The grouping is derived, not asserted. Five independent witnesses agree:

  • identical return shape { total, <x>ed, prop, counts: Record<…> };
  • both writers are (pid, guids, <enum>, publish) → editIfc;
  • both readers are consumed by apps/web/src/viewer/tools/modelStatePanels.ts (lines 251, 316);
  • both writers sit unwired and adjacent on apps/web/src/api/clientCallers.test.ts's UNCALLED
    allowlist;
  • model.ts already owned /model/lod/census, /lod/handover-readiness and /lod/assessment,
    while the base distribution /projects/{pid}/lod stayed behind — lodSummary was a sibling
    separated from its own family.

authoring_matrix.py disagrees, and it is recorded as the losing vote rather than elided. It
files set_lod under data and set_phase under lifecycle, because it categorises by the IFC
output each recipe writes — an LOD stage tag against Massing_Phasing.Status. Different property
sets, same question. That is (89)'s "storage is a HOW" trap: "they write different psets" has the
same shape as "they are all module records", and neither is a question. This is the first slice where
the matrix has been the losing vote after being right three running — worth naming, because three
straight wins is exactly how a corroborating source turns into an unexamined authority.

It meets (94)'s objection rather than overriding it. (94) declined setPhase because taking the
writer alone would have stranded phasing() in client.ts — the reader/writer split (87) had to
undo. Both halves move together here, so nothing is separated.

Scope claim, stated at the strength the evidence supports: these four answer that question and
belong together. This does not claim the question is now complete — no derivation of the complement
was done here, and the header says so.

The four names are added to apps/web/src/api/surface.test.ts because its floor is a slack ratchet
(788 actual vs a 751 floor), so the count alone would not notice a loss; and because the UNCALLED
allowlist is about call sites while that list is about the surface — an unwired writer can
still vanish in an extraction unnoticed.

Checklist (mirrors CONTRIBUTING.md)

  • Backend: cd services/api && python -m ruff check ../.. clean
  • Backend: affected test_*.py pass — test_file_sizes.py, test_claude_md_gates.py,
    test_roadmap_status.py all exit 0; full suite running as a cross-check
  • Web: typecheck, lint and build clean (npx tsc --noEmit = 0, npm run lint = 0,
    npm run build = 0); npx vitest run src/api = 27 files / 119 tests
  • No import cycles introduced
  • CHANGELOG.md entry added (newest at top); docs/roadmap.md updated
  • Version bump — n/a, not a release PR
  • No secrets, no competitor names in shipped docs

🤖 Generated with Claude Code

https://claude.ai/code/session_01Tt2XKB83wwNt2nrMbK6eEA


Generated by Claude Code

Summary by CodeRabbit

  • Refactor

    • Moved element state tools for LOD stages and construction phases to the model API.
    • Existing summaries and editing actions remain available through the updated model interface.
  • Documentation

    • Updated the roadmap and change records to reflect the API organization changes.
  • Tests

    • Updated API surface checks to cover the relocated element state methods.

…trix loses
Moves `lodSummary`/`setLod` and `phasing`/`setPhase` out of `client.ts` into
`api/model.ts`. They answer one question — *what state are the model's elements
in, and set it?* — and `client.ts` goes 727 -> 711, 76 methods above the STAYING
banner.
The grouping is derived, not asserted:
- identical return shape `{ total, <x>ed, prop, counts: Record<...> }`;
- both writers are `(pid, guids, <enum>, publish) -> editIfc`;
- both readers are consumed by `viewer/tools/modelStatePanels.ts` (251, 316);
- both writers sit unwired and *adjacent* on `clientCallers.test.ts`'s UNCALLED
allowlist;
- `model.ts` already owned `/model/lod/census`, `/lod/handover-readiness` and
`/lod/assessment`, while the base distribution `/projects/{pid}/lod` was left
behind in `client.ts` — `lodSummary` was a sibling separated from its family.
`authoring_matrix.py` DISAGREES and is recorded as the losing vote rather than
elided. It files `set_lod` under `data` and `set_phase` under `lifecycle`,
because it categorises by the IFC output each recipe writes — an LOD stage tag
against `Massing_Phasing.Status`. Different property sets, same question, which
is (89)'s "storage is a HOW" trap. This is the first slice where the matrix has
been wrong after being right three running.
It also MEETS (94)'s objection rather than overriding it: that slice declined
`setPhase` because taking the writer alone would have stranded `phasing()`, the
reader/writer split (87) had to undo. Both halves move together here.
The four names are added to `surface.test.ts` because its floor is a slack
ratchet (788 actual vs 751 floor) — the count alone would not notice a loss —
and because the UNCALLED allowlist is about call sites, not the surface.
Verified: tsc 0, lint 0, `vitest run src/api` 27 files / 119 tests, build 0,
`test_file_sizes.py` / `test_claude_md_gates.py` / `test_roadmap_status.py` /
ruff all 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tt2XKB83wwNt2nrMbK6eEA
@ibuilderClaude

Copy link
Copy Markdown
OwnerAuthor

@coderabbitai review


Generated by Claude Code

@coderabbitai

coderabbitaiBot commented Sep 4, 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: Team

Run ID: 17c968f0-54f9-415d-b431-51369822729c

📥 Commits

Reviewing files that changed from the base of the PR and between ca52ad6 and 44a6ed4.

📒 Files selected for processing (6)
  • CHANGELOG.md
  • apps/web/src/api/client.ts
  • apps/web/src/api/model.ts
  • apps/web/src/api/surface.test.ts
  • docs/roadmap.md
  • services/api/test_file_sizes.py

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


📝 Walkthrough

Walkthrough

The LOD and phasing read/write methods move from ApiClient to the Model mixin. Surface checks, documentation, changelog entries, and the client.ts file-size cap are updated.

Changes

Element-state extraction

Layer / File(s)Summary
Model element-state API
apps/web/src/api/model.ts
Adds lodSummary, setLod, phasing, and setPhase with typed responses and editIfc payloads.
Client extraction and surface coverage
apps/web/src/api/client.ts, apps/web/src/api/surface.test.ts
Removes the four methods from ApiClient, updates method-count notes, and adds the methods to surface checks.
Migration records
CHANGELOG.md, docs/roadmap.md, services/api/test_file_sizes.py
Records the extraction, updates progress totals, and lowers the client.ts size cap from 727 to 711.

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

Merge Risk:⚪ Minimal · up to 44a6e

LOD and phasing state APIs are reorganized into the model API while retaining their existing read and write behavior. No concrete current-head merge risk remains.

Suggested reviewers:cursoragent

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files. (2 skipped: 2 …Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Description check✅ PassedThe description explains the four-method extraction, its rationale, scope, validation results, documentation updates, and checklist status. It is complete and aligned with the repository template.
Title check✅ PassedThe title clearly identifies the element-state extraction and the grouping rationale. It is specific, concise enough, and related to the main change.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/codebase-audit-roadmap-45656d

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai

coderabbitaiBot commented Sep 4, 2026

Copy link
Copy Markdown

@ibuilder: I will review pull request #412.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ibuilder
ibuilder marked this pull request as ready for review September 4, 2026 04:29
@strix-security

Copy link
Copy Markdown

Strix is installed on this repository, but we couldn't run this PR security review because this workspace's trial has ended. Add a card to resume code reviews here.

So far, Strix has reviewed 28 pull requests, surfaced 3 security issues (1 critical/high) and blocked 2 risky merges across this workspace.

@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@ibuilder
ibuilder merged commit b0e8281 into mainSep 4, 2026
12 checks passed
ibuilder pushed a commit that referenced this pull request Sep 4, 2026
PR #412 was squash-merged, so origin/main carries (95) as b0e8281 while this
branch still pointed at the pre-squash 44a6ed4. The content diff between the
remote branch and origin/main was empty before this merge, so the branch held
no unmerged work; -s ours keeps this branch's tree (origin/main plus (96)) and
reconciles the histories without rewriting the remote branch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tt2XKB83wwNt2nrMbK6eEA
ibuilder pushed a commit that referenced this pull request Sep 4, 2026
PRs #412, #413, #414, #431, #432 and #433 were all squash-merged, so origin/main
carries their work as new commits while this branch still held the pre-squash
originals. Verified before merging: the content diff between the remote branch
and origin/main is empty, and so is the diff between this branch and
origin/main — every slice is already on main and nothing is unmerged on either
side.
-s ours keeps this branch's tree (identical to origin/main) and reconciles the
histories so the branch can fast-forward push, without rewriting the remote
branch's commits.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tt2XKB83wwNt2nrMbK6eEA
Sign up for freeto 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

@ibuilder@claude