chore(#973): partition the 48 OCAF-family packages that belonged to no pass - #987
Merged
Conversation
…o pass #807's characteristic defect is a package that belongs to no pass, and #973 is that defect at package scale: 48 packages and 459 headers of the pinned kernel were named by no sub-issue of the refman-coverage epic. `Scripts/repro/973-ocaf-package-partition/partition_census.py` is the census-rule artifact for the partition. It enumerates every OCAF-family package in the pinned kernel, prints which pass owns it, and exits 1 if any has no owner, so the set can be re-checked rather than re-derived by hand. The family is derived rather than chosen, in four mechanical tiers: OCCT's own `ApplicationFramework` module (43 packages, 509 headers), packages outside it whose every OCCT consumer is an OCAF package (7), the XDE attribute layer already in Pass 3's lane (6), and five packages that reached #973's own table only by matching a `Bin*`, `Xml*` or `Std*` prefix and are not OCAF at all. The ownership decisions are landed in the issue bodies, not only here: two new passes were filed (#982 Pass 3b, the OCAF framework layer, 5 packages and 51 headers; #983 Pass 3c, OCAF persistence and format drivers, 38 packages and 342 headers), #813 and #814 gained two packages each, #810's `## Lane` was corrected to name the six packages its census already claims, and `StdFail_` was recorded as belonging to no lane on purpose. `--verify-lanes` fails if a `## Lane` and this table stop agreeing. Re-derived rather than inherited. #973's title says 44 packages and ~460 headers; its table lists 43 rows summing to 410. Every per-package count in that table is correct and five whole packages are missing from it (`LDOM` 24, `ShapePersistent` 13, `UTL` 1, `FSD` 7, `Plugin` 4), so 410 + 49 = 459, which is the prose figure. The prose header total was right and the table under it was not; the prose package count was wrong against both. Two corrections to #973's own suggestions, both measured: `Resource_` is Pass 4c's rather than Phase 6's (it has a public `ResourceManager` Swift type, thirteen operations in `docs/API_REFERENCE.md`, a bridge home in `OCCTBridge_IO.mm`, and twelve of its seventeen OCCT consumers are STEP, IGES and Units), and folding the persistence layer into #813 was rejected because that lane is 192 headers and the layer is 342. The removal and injection matrices are in the directory's README. One injection found a real hole: deleting a package from the table exited 0, because the prefix sweep printed a note rather than failing. It fails now, backed by an explicit exclusion table. Closes #973 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rejected-alternative figure said folding the persistence layer into #813 would produce a pass 84% larger than the largest in the epic. That compared 512 against #810's 278, which is not the largest: Pass 4d is 329 headers, and 368 after gaining StdPrs_ and StdSelect_ from this same partition. Re-measured, the correct statement is 192 + 342 = 534, nearly three times Pass 4c's own size and 45% larger than Pass 4d. The conclusion is unchanged; the number that supported it was taken against the wrong neighbour, which is the failure okf/policies/measure-dont-assume.md names. #983's and #813's issue bodies carried the same figure and are corrected too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Previous Review Summary (commit 5a1ebee)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 5a1ebee)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Reviewed by nemotron-3-ultra-550b-a55b:free · Input: 287.2K · Output: 4.9K · Cached: 777.6K |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| return [c for c in classes if re.search(r"\b" + re.escape(c) + r"\b", haystack)] | ||
|
|
||
|
|
||
| NAMED_IN_LANE = re.compile(r"`[^`\n]*") |
There was a problem hiding this comment.
SUGGESTION: Unused variable NAMED_IN_LANE
The regex NAMED_IN_LANE = re.compile(r"[^\n]*") is defined on line 397 but never referenced anywhere in the file. It appears to be dead code and should be removed.
Suggested change
| NAMED_IN_LANE = re.compile(r"`[^`\n]*") |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
#807's characteristic defect is a package that belongs to no pass: #382 left three Wire files
unclaimed for weeks, #384 left seven assembly files unclaimed until challenged, #377 records three
files holding the repo's highest-scoring duplication that belong to no pass at all, and #810's
census found six packages in its own lane that no sub-issue named. #973 is that defect at package
scale, and this PR is the partition that ends it.
48 packages, 459 headers of the pinned kernel belonged to no pass. They now have owners, and
Scripts/repro/973-ocaf-package-partition/partition_census.pyis the census-rule artifact thatenumerates every OCAF-family package, prints which pass owns it, and exits 1 if any has none.
This is issue-body and artifact work. No
Sources/change, so no build.Closes #973
The partition
61 packages, 737 headers, 142 classes reached from the bridge.
The decision is landed where it takes effect, not only recorded. Done outside this diff with
gh issue edit, as #973 asks:TFunction_,TPrsStd_,TObj_,AppStd_,AppStdL_. OCAF proper,above the document API and below the drivers. Folding into Refman coverage audit: Document/XDE assembly (Pass 3) #810 was rejected: that pass's audit
is complete and its PR (fix(#810): refman coverage audit for Document/XDE assembly (Pass 3) #977) open, so anything added now sits inside a finished lane and is
audited by nothing.
itself proposed, was rejected on measurement: after this partition Refman coverage audit: Export and interop lane (Pass 4c) #813's lane is 192 headers and
this layer is 342, so folding would take Pass 4c to 534, nearly three times its size and 45%
larger than the largest pass in the epic (Pass 4d at 368).
## Lanecorrected to name the six packages its census already claims(
TDataXtd_,TNaming_,XCAFDimTolObjects_,XCAFNoteObjects_,XCAFPrs_,XCAFView_), soa cold reader sees the scope the artifact audits, plus a "not in this lane" paragraph naming
Refman coverage audit: OCAF framework layer (Pass 3b) #982 and Refman coverage audit: OCAF persistence and format drivers (Pass 3c) #983.
## LanegainsBinTools_andResource_, with the measurement for each.## LanegainsStdPrs_andStdSelect_, and records both theStdSelect_/Refman coverage audit: Selection and Construction (Pass 2b) #809overlap and that
SelectMgr_is unowned and out of this scope.partition; both new issues are linked as real sub-issues.
Re-derived, not inherited
#973's title says 44 packages and ~460 headers; the table in its body lists 43 rows summing to 410.
--reconcile-973prints the comparison:LDOM(24),ShapePersistent(13),UTL(1),FSD(7),Plugin(4). The first three areApplicationFrameworkpackages aprefix-shaped derivation cannot see, because their names do not begin
Bin,Xml,Std,TorApp.under it was not, the reverse of the usual direction. The prose "44" was wrong against both.
LDOMalso shows why the split needs care: 3 of its 24 headers (LDOMParser.hxx,LDOMString.hxx,LDOMBasicString.hxx) do not carry the package underscore, so a naive split reports 21. There areexactly four such headers in the kernel and
--reverify-familyfails if a fifth appears.Two corrections to #973's own suggestions
Resource_is Pass 4c's, not Phase 6's. Refman coverage: 44 OCAF-family packages (~460 headers) belong to no pass #973 guessed Phase 6 because OCAF's formatregistration uses it. Measured: a public
ResourceManagerSwift type, thirteen operations indocs/API_REFERENCE.md, a bridge home inOCCTBridge_IO.mm, and twelve of its seventeen OCCTconsumers outside OCAF being
DESTEP_,DEIGES_,STEPControl_,STEPCAFControl_,StepData_,XSAlgo_,ShapeProcess_andUnits_.Resource_Unicode::SetFormatis the STEP and IGES textencoding switch specifically. Two independent constructions agree.
Bin*/Std*prefix:StdFail_(the kernel exception vocabulary),StdPrs_andStdSelect_(Visualization/TKV3d),BinTools_(ModelingData/TKBRep). They are kept in the table with that recorded, becausedeleting them would put them straight back into being owned by nobody.
The one package that belongs to no pass, on purpose
StdFail_. 56 OCCT packages reference it, every one outside OCAF and spread across every lane, sono lane has a better claim than any other. Nothing wraps it as a capability: it is what the bridge
catches, not what it calls, and the only claims
docs/makes about it are convention statements indocs/naming-conventions.md. Recorded as #820's, and--verify-lanesfails if any lane startsnaming it, which is the inverted form of the same check.
CHANGELOG entry
Every OCAF-family package now belongs to a pass of the refman-coverage epic (#973)
Forty-eight packages and 459 headers of the pinned kernel were named by no sub-issue of #807, the
refman-coverage epic, and would have arrived unannounced at its final whole-surface pass. They are
now partitioned, and the partition is a committed, re-runnable artifact:
Two new passes were filed for the bulk of it, #982 (the OCAF framework layer:
TFunction_,TPrsStd_,TObj_,AppStd_,AppStdL_) and #983 (OCAF persistence and format drivers:PCDM_,Storage_,FSD_,Plugin_,LDOM_and everyBin*/Xml*/Std*driver package). Pass 4c(#813) gained
BinTools_andResource_, Pass 4d (#814) gainedStdPrs_andStdSelect_, andPass 3's (#810) lane text was corrected to name the six packages its own census already claims.
StdFail_is recorded as belonging to no lane pass on purpose, with its reason.The census exits 1 on any family package with no owner, on a header count that has drifted from the
pinned kernel, and, under
--verify-lanes, on an owning issue whose## Laneno longer names whatit owns. No
Sources/ordocs/change: this is ownership, not coverage, and each owning passstill runs its own per-class audit.
SemVer impact
NONE. No public API, no behaviour, no
Sources/and nodocs/change. The diff is one newScripts/repro/directory.Checklist
verification), see SecondMouseAU/OCCTReconstruct#397
for the ecosystem-wide test-coverage standard this is piloting.
--self-testcase was run once with its subject broken, and thefailure is reported here, see okf/policies/prove-the-test-fails.md.
docs/CHANGELOG.mdis not in this diff.docs/SEMVER.mdis not in this diff.It is assessed at release on
main, not per PR.Notes for the reviewer
Prove the test fails: both matrices were run, and both found something
Full transcripts are in
Scripts/repro/973-ocaf-package-partition/README.md. Summary:Removal matrix (each accepting or rejecting shape in the two detectors, 15 self-test cases):
[^`\n]*walk inside the backtick run(?:_|\*|\b)tail assertionOWNERSR2 initially passed, so the prefix walk was decorative and a case was added that exercises it.
No
## Laneis written that way today, so that one synthetic case is the only thing holding theshape; the README says so rather than leaving the next reader to infer it. R5's original single
form is what became R5a, and it proved nothing on its own.
Injection matrix (each guard in the run modes):
TDFbaked as 52 headersTDF: baked 52 headers, pinned kernel has 51Storagegiven an unknown ownerStdStoragedeleted from the tableTObjmarked as not in the modulemembership drifted: only in kernel ['TObj']StdPrsassigned to #812StdFailassigned to #814I3 is the real finding. Deleting a package from the table exited 0: the prefix sweep printed it
as a note and the run passed. A census whose own failure mode is "a package belongs to no pass"
cannot report all clear when a package leaves its table, so the sweep is now a failure backed by an
explicit
PREFIX_SWEEP_EXCLUSIONStable (empty today). Re-run: exit 1,1 prefix-matching package(s) in neither FAMILY nor PREFIX_SWEEP_EXCLUSIONS: StdStorage.Gates
All seventeen
gate-scriptsinvocations fromci.ymlrun locally and green, plus this artifact'sown two (
--self-test, plain run).Trade-offs and known limits, stated rather than left to be found
--verify-lanesis forward only. Every package assigned to issue N must be named in N's## Lane; a second issue also naming it is reported, not failed, because a lane legitimatelynames its neighbours (Refman coverage audit: OCAF framework layer (Pass 3b) #982's lane explains
AppStd_Applicationin terms ofTDocStd_Application,which is Refman coverage audit: Document/XDE assembly (Pass 3) #810's). Sixteen such extra namings exist today and every one is cross-reference prose
written by this issue. A genuine double claim is visible in that report but is not caught
automatically.
--reverify-familyneedsLibraries/occt-src, a gitignored build artifact, so it cannot runin CI. The prefix sweep in the plain run is the weaker form that works from shipped headers alone.
The plain run needs
Libraries/OCCT.xcframeworkand exits 2, not 1, without it: a missingxcframework is not a finding about the tree.
--globalreports 248 shipped packages, 4771 headers, named byno lane at all. That is Phase 6's (Refman coverage audit: whole surface, and did the eleven lanes partition it (Phase 6) #820) subject and out of scope here; the README says why it is
not a work list.
SelectMgr_is the one a reader of this partition is most likely to ask about,and Refman coverage audit: Mesh, presentation and misc lane (Pass 4d) #814's
## Lanerecords that it is unowned.StdSelect_is the one boundary where two passes have a claim, since theSelectionSwiftsurface is Refman coverage audit: Selection and Construction (Pass 2b) #809's. Filed with Refman coverage audit: Mesh, presentation and misc lane (Pass 4d) #814 because the OCCT classes are TKV3d and the bridge site is the
AIS one; Refman coverage audit: Mesh, presentation and misc lane (Pass 4d) #814's lane records the overlap.
Refman coverage audit: OCAF persistence and format drivers (Pass 3c) #983's by construction.