fix(core,geometry,export): collapse three STEP comment-skip rules that disagreed on an unterminated /*

[BIMvoice]

Summary

Fixes #3303. Three places in rust/ hand-rolled "skip a STEP /* ... */ comment" and answered an unterminated /* three different ways:

	unterminated /*
rust/core EntityScanner (scanner.rs)	refused (returned None)
rust/geometry CoordIndex reader (processors/mod.rs)	silently consumed to end of input
rust/export header prescan (source_header.rs)	treated as ordinary text, costing one /

• Basis for the chosen answer: the scanner and the geometry reader are both walking already-located, well-formed record bytes looking for structure, not doing a header prescan — an unterminated comment there means the input is corrupt, and silently consuming the rest of it is worse than refusing. Neither had a reason to differ from the other, so this collapses them onto one rule, ifc_lite_core::skip_step_comment, which refuses.
• Collapsed, not pinned: both rust/geometry and rust/export already depend on ifc-lite-core (checked Cargo.toml before touching anything), so there was no crate-dependency-direction reason to keep three copies. rust/geometry's CoordIndex reader now calls the shared function and refuses, matching the scanner, instead of reading past a corrupt comment.
• rust/export keeps its own different answer, on purpose: its STEP HEADER prescan (Lex::skip_comment_at) treats an unterminated /* as ordinary text rather than refusing, because a header prescan that swallows every later record has lost the schema — worse than the malformed input deserves. That reasoning predates this change and is documented on the type. It now finds a closed comment's end via the same shared skip_step_comment too; only the unterminated case stays its own, and the doc comment says why.
• Fourth copy: grepped rust/ and packages/ for in_string/unterminated/skip_comment/skipLexical and found none. Every other STEP comment/string-skip site already delegates to one of the three: rust/export/src/schema_detect.rs's find_unquoted already reuses source_header::Lex::skip_lexical_at, and the TypeScript side's packages/parser/src/step-lexing.ts is the only place either of tokenizer.ts or source-header.ts calls for this rule. schema_detect.rs does carry a comment noting a separate, unrelated divergence (case-sensitive ENDSEC/FILE_SCHEMA matching vs the TS side) that it says is "tracked in Three Rust crates hand-roll the STEP lexical rule, and they answer an unterminated comment three different ways #3303" — that is not the comment-skip rule this issue is about and is left untouched here.

RED before the fix

rust/geometry/src/processors/tests.rs gained a test comparing the CoordIndex reader's pre-fix inline comment-skip (skip_comment_lossy, since removed) against ifc_lite_core::EntityScanner on the same unterminated-comment fixture:

thread 'processors::tests::unterminated_comment_geometry_consumes_to_end_today' panicked at rust/geometry/src/processors/tests.rs:1322:5:
ifc-lite-geometry and ifc-lite-core must agree that an unterminated comment is refused, not silently consumed; today they do not

Test plan

• cargo test -p ifc-lite-core -p ifc-lite-geometry -p ifc-lite-export — 114 test-result blocks (unit + integration + doctests across all three crates), 0 failed, EXIT=0. Run twice: once that hit a shared-target-dir race from a concurrent build (unrelated E0425 on a stale cached rlib, resolved by rebuilding), and once clean with an isolated CARGO_TARGET_DIR.
• rust/core/src/parser/lexical.rs's own unit tests, rust/geometry/src/processors/tests.rs::unterminated_comment_geometry_and_core_scanner_agree, and rust/export's source_header_comments.rs::an_unterminated_comment_does_not_swallow_the_records_after_it all pass.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com

https://claude.ai/code/session_01QPHChk3Ve9N519A4kY7436

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of unterminated STEP block comments.
  • Geometry coordinate data now rejects malformed comments instead of reading past the end of the input.
  • Comment handling is now consistent across entity scanning, geometry processing, and header parsing.

• Tests

  • Added regression coverage to verify consistent rejection of unterminated comments.

[coderabbitai]

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d20d80e4-f45d-4383-825d-100d1a35844b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 69d6ac45-8516-4234-88d0-afe8fcb59ecf

📥 Commits

Reviewing files that changed from the base of the PR and between 5a431e5 and 9d5d1f9.

📒 Files selected for processing (8)

• .changeset/collapse-step-comment-skip.md
• rust/core/src/lib.rs
• rust/core/src/parser/lexical.rs
• rust/core/src/parser/mod.rs
• rust/core/src/parser/scanner.rs
• rust/export/src/source_header.rs
• rust/geometry/src/processors/mod.rs
• rust/geometry/src/processors/tests.rs

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

---

📝 Walkthrough

Walkthrough

The PR adds a shared Rust helper for STEP block comments. Entity scanning and geometry processing reject unterminated comments. Header prescanning retains its distinct handling while using the helper for closed comments. Tests cover the shared behavior.

Changes

STEP comment handling

Layer / File(s)	Summary
Shared comment-skip rule rust/core/src/parser/lexical.rs, rust/core/src/parser/mod.rs, rust/core/src/lib.rs	Adds and tests skip_step_comment. The function returns the position after a closed comment and None for non-comments or unterminated comments. The function is re-exported from the core crate.
Consumer integration and regression coverage rust/core/src/parser/scanner.rs, rust/geometry/src/processors/mod.rs, rust/export/src/source_header.rs, rust/geometry/src/processors/tests.rs, .changeset/collapse-step-comment-skip.md	Entity scanning and geometry processing use the shared helper. Header prescanning uses it for closed comments and preserves its existing unterminated-comment behavior. A regression test checks agreement between the core scanner and geometry processing.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 9d5d1

This change consolidates STEP comment handling and adds coverage for unterminated comments; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: louistrue

Poem

A rabbit found comments deep in the hay

And taught every scanner the same careful way
Closed marks hop onward, two bytes at a time
Open-ended marks stop before crossing the line
Core, geometry, and headers now agree>

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes satisfy issue #3303 [#3303]. They add a shared core helper, update the core scanner and geometry reader, preserve export's intentional header-prescan behavior, and add regression coverage.…
Out of Scope Changes check	✅ Passed	All changes are within scope. The helper, call-site updates, export integration, regression test, public re-export, and changeset directly support the stated consolidation objective.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 7 files. (1 skipped: 1…
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: consolidating STEP comment-skipping behavior across the core, geometry, and export crates, with focus on unterminated comments.

Full details: Linked Issues check

Explanation

The changes satisfy issue #3303 [#3303]. They add a shared core helper, update the core scanner and geometry reader, preserve export's intentional header-prescan behavior, and add regression coverage. Issue #39 has no relevant requirements.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 7 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

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

[github-actions]

Viewer benchmark

⚠ 1 metric(s) exceeded the regression threshold (advisory only, not blocking).

01_Snowdon_Towers_Sample_Structural(1).ifc

Baseline recorded 2026-07-01T20:31:05.538Z on github-actions ubuntu-latest, viewer-benchmark-ci (headless Chrome, SwiftShader ANGLE), production build.

Metric	Current	Baseline	Delta	Threshold	Status
firstBatchWaitMs	2383ms	2905ms	-18.0%	+50%	✅
firstVisibleGeometryMs	2970ms	3652ms	-18.7%	+50%	✅
streamCompleteMs	3422ms	3598ms	-4.9%	+50%	✅
spatialReadyMs	1084ms	1032ms	+5.0%	+50%	✅
metadataCompleteMs	1535ms	3063ms	-49.9%	+50%	✅
totalWallClockMs	3500ms	3700ms	-5.4%	+50%	✅

AC20-FZK-Haus.ifc

Baseline recorded 2026-07-01T20:30:59.972Z on github-actions ubuntu-latest, viewer-benchmark-ci (headless Chrome, SwiftShader ANGLE), production build.

Metric	Current	Baseline	Delta	Threshold	Status
firstBatchWaitMs	291ms	1075ms	-72.9%	+50%	✅
firstVisibleGeometryMs	2602ms	1572ms	+65.5%	+50%	❌
streamCompleteMs	1030ms	1980ms	-48.0%	+50%	✅
spatialReadyMs	1060ms	915ms	+15.8%	+50%	✅
metadataCompleteMs	1155ms	1392ms	-17.0%	+50%	✅
totalWallClockMs	2700ms	3300ms	-18.2%	+50%	✅

Refresh the baseline from a CI run: dispatch the Benchmark workflow with record_baseline, download the benchmark-baseline artifact, and commit baseline.json (see tests/benchmark/README.md).

[louistrue]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

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.