SCALE-SEAM (96) — the as-built question's aggregate reader, and a witness that actually bounds - #413
Merged
Merged
Conversation
…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…ness that actually bounds Moves `lod500`, `setManufacturerInfo` and `attachOmDocument` out of `client.ts` into `api/model.ts`, rejoining `verifyAsbuilt` and `recordAsbuiltDimension` which (94) moved. `client.ts` goes 711 -> 698, 73 methods above the STAYING banner. TWO BOUNDING WITNESSES, derived independently, agreeing: - `openAsBuiltPanel` in `viewer/tools/modelStatePanels.ts` calls exactly five API methods — read off that function's brace closure rather than grepped for, so it is the closure of a scope and not a sample. Two of the five were already in `model.ts`. - The reader's own response type names its writer set field by field: `verified`/`by_method` from `verify_asbuilt`; `with_dimensions`/`dimensions_out_of_tolerance` from `record_asbuilt_dimension`; `with_manufacturer`/`with_serial` from `set_manufacturer_info`; `with_om_docs`/`om_documents` from `attach_om_document`. The backend route says it in prose: "Stamp elements with the `verify_asbuilt` recipe." A reader whose response type enumerates its writers is a DERIVED population. After eight slices of sampling, that is the first grouping witness here that bounds a set instead of illustrating one. WHAT CORROBORATES IS NOT WHAT BOUNDS. `test_lod500.py` reaches exactly three recipes — `attach_om_document`, `set_manufacturer_info`, `verify_asbuilt` — and OMITS `record_asbuilt_dimension`, which is unambiguously in this family. It agrees with the answer without being able to establish it. Three slices claimed "and no others" off a test file and were wrong all three times. NOT CLAIMED, and said in the header, the pin, the changelog and the roadmap rather than only in review: `attachDocument` stays in `client.ts`, takes a `purpose` parameter, and `asbuilt_summary` counts ANY purpose-tagged document reference — so "every writer of `with_om_docs` moves here" is false. The field map is of the recipes each field was designed around, not of everything that can set it. Two sources disagree and lose for the same reason: `attach_om_document` is a purpose-tagged wrapper of `detailing.attach_document`, and `authoring_matrix.py` files it and `set_manufacturer_info` under `data`. The first is a shared HELPER, the second a STORAGE bucket — the groupings (89) and (90) each had to reject. The matrix is the losing vote twice running after being right three times. Also fixes the extraction helper: it terminated its brace count on the method signature line, so a signature wrapping across two lines with no opening brace on the first read as a 3-line method. `attachOmDocument` is 6. Verified: tsc 0, lint 0, `vitest run src/api` 27 files / 119 tests, build 0, ruff (the CI command, from `services/api`) 0, and `test_file_sizes.py`, `test_claude_md_gates.py`, `test_roadmap_status.py`, `test_ruff_scope.py`, `test_reachable.py` all 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tt2XKB83wwNt2nrMbK6eEA
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
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThree API methods moved from ChangesModel API extraction
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk:⚪ Minimal · up to This change relocates three as-built API methods while retaining their client-facing availability and associated behavior. No concrete merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation 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.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
ibuilder
commented
Sep 4, 2026
OwnerAuthor
@coderabbitai review Generated by Claude Code |
✅ Action performedReview finished.
|
ibuilder
marked this pull request as ready for review
September 4, 2026 05:02
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 reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
Uh oh!
There was an error while loading. Please reload this page.
ibuilder pushed a commit
that referenced
this pull request
Sep 4, 2026
PR #413 was squash-merged, so origin/main carries (96) as ffb7b9f while this branch still held the pre-squash commits. 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 (97)) 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
7 tasks
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
apps/web/src/api/client.tsgoes 711 → 698 (73 methods above the STAYING banner, 4 below).Three methods move to
apps/web/src/api/model.ts—lod500,setManufacturerInfo,attachOmDocument— rejoiningverifyAsbuiltandrecordAsbuiltDimension, which (94) moved andcould only claim as "two known members, not necessarily all". This slice can say more than that,
and the reason matters more than the extraction. Continues SCALE-SEAM in
docs/roadmap.md.Two bounding witnesses, derived independently, agreeing
openAsBuiltPanelinapps/web/src/viewer/tools/modelStatePanels.tscalls exactly five APImethods — read off that function's brace closure rather than grepped for, so it is the closure of a
scope and not a sample. Two of the five were already in
model.ts.Separately, the reader's own response type names its writer set field by field:
verified,by_method,readiness_pctverify_asbuilt— moved in (94)with_dimensions,dimensions_out_of_tolerancerecord_asbuilt_dimension— moved in (94)with_manufacturer,with_serialset_manufacturer_info— moves herewith_om_docs,om_documentsattach_om_document— moves hereThe backend route states it in prose: "Stamp elements with the
verify_asbuiltrecipe."A reader whose response type enumerates its writers is a derived population. After eight slices
of sampling, that is the first grouping witness in this sequence that bounds a set rather than
illustrating one.
What corroborates is not what bounds
services/api/test_lod500.pyreaches exactly three recipes —attach_om_document,set_manufacturer_info,verify_asbuilt— and omitsrecord_asbuilt_dimension, which isunambiguously in this family. It agrees with the answer without being able to establish it. A
witness that misses a known member cannot bound anything, however exactly its members match.
Three slices claimed "and no others" off a test file and were wrong all three times; the fix is not
to read the test harder, it is to ask which question the test is able to answer at all.
What is NOT claimed
attachDocumentstays inclient.ts, takes apurposeparameter, andasbuilt_summarycountsany purpose-tagged document reference — so "every writer of
with_om_docsmoves here" isfalse. The field map above is of the recipes each field was designed around, not of everything
that can set it. The claim that survives is the bounded one: the five methods
openAsBuiltPanelcalls. This caveat is in the header, the size pin, the changelog and the roadmap — not only here.
(94) hedged in its PR while its artifact overstated; that is what this is avoiding.
Two sources disagree, and lose for the same reason
attach_om_documentis implemented as a purpose-tagged wrapper ofdetailing.attach_document, andauthoring_matrix.pyfiles it andset_manufacturer_infounderdata. The first is a sharedhelper, the second a storage bucket — the two groupings (89) and (90) each had to reject.
The matrix is now the losing vote twice running after being right three times; a corroborating
source that keeps winning is the one that quietly stops getting checked.
Also fixed
The extraction helper terminated its brace count on the method signature line, so a signature that
wraps across two lines with no opening brace on the first read as a 3-line method.
attachOmDocumentis 6.Note on this branch's history
#412 was squash-merged, so
origin/maincarries (95) asb0e8281dwhile this branch still pointedat the pre-squash
44a6ed44. The content diff between the remote branch andorigin/mainwasempty, so the branch held no unmerged work; the histories were reconciled with a
-s oursmergerather than a force-push, leaving the remote branch's commits intact.
Checklist (mirrors CONTRIBUTING.md)
cd services/api && python -m ruff check ../..cleantest_file_sizes.py,test_claude_md_gates.py,test_roadmap_status.py,test_ruff_scope.py,test_reachable.pyall exit 0; full suite running as a cross-checknpx tsc --noEmit0,npm run lint0,npm run build0;npx vitest run src/api27 files / 119 testsCHANGELOG.mdentry added (newest at top);docs/roadmap.mdupdated🤖 Generated with Claude Code
https://claude.ai/code/session_01Tt2XKB83wwNt2nrMbK6eEA
Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation