Repository navigation
fix(census): settle the class-count shortfalls against AutoCAD's DXF export (#230) - #296
Merged
Merged
Conversation
…export (#230) probe_class_census gains --twin: the export's records of each class, how many carry a handle that is no object of the drawing, and the export's own CLASSES count. ACDB_TEXTOBJECTCONTEXTDATA_CLASS on the R2004/R2007 saves and GEODATA and AcGeoBackupObject on geoloc.dwg are short of the DWG's class count, and the export holds exactly what the walk reaches, while the two class tables disagree with each other. They join the allowlist. With --twin, an allowlisted shortfall is excused only when the export agrees, and a missing twin fails the run.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8cab4f746
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
# Conflicts: # ARCHITECTURE.md
3 tasks done
…malformed DXF twin (#230) The export-only column was built from the objects the walker returned, so a record the walker skipped counted as data the export adds and subtracted itself from the independent census: with the sole GEODATA skipped, the walked count and the adjusted twin count were both zero and the shortfall was excused. The set now comes from the drawing's handle map, which the walk cannot shrink, and a run without one fails. read_twin accepted any file and returned a census of zero for an empty or truncated export, which excuses every allowlisted shortfall. It now refuses a file whose lines are not whole group pairs, whose group codes are not numbers, whose sections do not open and close in order, that lacks an ENTITIES or an OBJECTS section or its closing EOF, or that has data after the EOF. The crate's dxf_reader does this too but is behind a feature the CI builds of the probe do not enable.
2 tasks done
DrunkOnJava
added a commit
that referenced
this pull request
Oct 28, 2026
* fix(ci): hold a stacked PR until the older PR it builds on lands A PR stacked on an older open PR targets main but holds that PR's commits. When the train held the older PR on a review thread it went on to the next one: landed first, that PR would have carried the older one's work into main under its own title (#337 over #335, #339 over #337 were drafted by hand to stop it), and its refresh merged main over the older PR's commits, so after that PR landed its merge base no longer applied and a lane had to merge it by hand. The train now sends such a PR to the end of the queue while an older open PR (a lower number) shares commits with it that main does not have. On the queue of 2026-10-28: #324 waits for #323, #355 for #324, #356 for #355, #410 for #356, #412 for #410, #339 for #337 and #341 for #339; #323, #337, #389 and #296 wait for nothing. * fix(ci): check every open PR, skip landed work, and defer on a failed lookup The stack check listed at most 200 open PRs, only those targeting main, treated any shared commit as outstanding even when it belonged to a PR that had already landed by squash, and let a PR through when a fetch or gh call failed. It now fetches every PR head to refs/train/pr/<n> in the main clone, pages through all open PRs whatever they target, ignores a shared commit that a merged PR's head holds, and returns 2 on any failed lookup, which sends the PR to the end of the queue. Checked in a scratch repository (two PRs built on a landed PR do not wait for each other; a PR built on another waits for it; a failing gh returns 2) and on the live queue, where the R13 and unp stacks give the same order as before. * fix(ci): count only PRs merged into main as landed when checking a stack A PR merged into a feature branch has merged_at set too, so its commits were treated as landed and a newer PR holding them went ahead of the open PR they were waiting in. The exemption now takes only PRs whose base is main. All 187 merged PRs so far target main, so the live order is unchanged.
# Conflicts: # ARCHITECTURE.md
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
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.
Summary
Settle #230's three class-census shortfalls against AutoCAD's own DXF export of each drawing.
examples/probe_class_censusgains a--twin <file.dxf>mode. For each class it counts the export's records, how many of those carry a handle that is no object of the drawing (export-only), and the instance count the export's own CLASSES section gives the class.On every shortfall the export holds no more records than the walk reaches. The DWG's class table and the export's class table also disagree with each other. So the count is a stale registration figure rather than a census, and ACDB_TEXTOBJECTCONTEXTDATA_CLASS, GEODATA and AcGeoBackupObject join the allowlist with that evidence. With
--twin, an allowlisted shortfall is excused only when the export agrees for that file, and a missing twin fails the run.Closes #230.
The per-release counts
sample_AC1018.dwg(R2004)sample_AC1021.dwg(R2007)sample_AC1024.dwg(R2010)sample_AC1027.dwg(R2013)sample_AC1032.dwg(R2018)geoloc.dwg(R2018)geoloc.dwg(R2018)The allowlisted TABLECONTENT and TABLEGEOMETRY (#56) say the same thing on every ACadSharp save from R2004:
The twin rule, on the whole corpus
All 42 drawings of the public corpus that have a DXF twin pass
--twin. The rule excuses an allowlisted shortfall only when the export holds no more records of the class than the walk reached, not counting its export-only ones.The only export-only surplus is CELLSTYLEMAP on the seven nextgis R2010 saves. The export gives TABLESTYLE
0x65an xdictionary0xD2holding a round-trip CELLSTYLEMAP0xD3. Neither handle is an object of the DWG, andprobe_reference_closurefinds no record of the drawing referring to a handle it does not hold. That is the same kind of round-trip data the export adds to LTYPEGAS_LINE(#293).The export also gives one of the two TABLECONTENT and TABLEGEOMETRY records of each ACadSharp save a handle the DWG does not use. That is why the rule asks the walk only not to fall short of the export, and never to match it exactly.
Negative checks:
--twinwith a missing file exits 1 ("cannot read the DXF twin").--twinwith no path exits 1.geoloc.dxfwith a second GEODATA record under an existing drawing handle (1F) is no longer excused. The probe reportsRESULT: at least one class is under-walked.Without
--twin, the probe's output is byte-for-byte what it was.measure.py's census parser and the canonical-fixture--strictrun in CI see no change.Coverage and baseline
No decoder changes. Coverage is unchanged,
tools/corpus/baseline.tsvis unchanged, andmeasure.py checkpasses against it. The census column still lists every shortfall, sincemeasure.pyrecords them regardless of the allowlist.Docs
probe_class_census.rs: the module docs describe--twinand the rule, andALLOWLIST's docs and reasons carry the twin evidence.Test plan
cargo fmt --all -- --checkcargo clippy --all-targets --all-features -- -D warningsRUSTFLAGS="-D warnings" cargo test --release --all-featuresRUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-featurestools/corpus/measure.py checkpasses against the unchanged baselinetests/class_census.rsalready pins the counts per file.Source-provenance declaration
By submitting this PR I certify that my contribution does not incorporate executable code from any source whose license is incompatible with Apache-2.0. Specifically:
CLEANROOM.md).My contribution is based on §10 (
AcDb:Classes) of the ODA's freely-redistributable Open Design Specification for .dwg files (v5.4.1) and on the corpus bytes. It also uses AutoCAD's DXF exports of the same drawings, read as data files (ACadSharpsamples/atc30a97ce, MIT, and nextgis/dwg_samples, pinned by #225). No third-party code or comments were read.Developer Certificate of Origin
By submitting this PR, I certify that the contribution complies with the Developer Certificate of Origin 1.1.