Commit afb9725
authored
## Reviewer summary
- **Without this:** the same truncated/corrupted-file data loss #3695 fixed on the TypeScript path is still silent on the Rust (WASM/native) load path — a model can lose its entire tail with zero signal, and which load path a user happened to hit decided whether they were ever told their model was incomplete.
- **Evidence:** on the unmodified scanner, a fixture with a valid entity, an unterminated-string entity, then another valid entity scans to `left: None, right: Some(...)` — the malformed record was silently dropped instead of attributed. The PR's own framing: "the malformed record must be attributed, not silently dropped."
- **Risk:** the sharded parallel-scan pre-pass needed care to avoid conflating a genuine terminal stop with a mid-string artifact from a shard's speculative start; a chunk-count sweep (`[1, 2, 3, 4, 5, 7, 8, 11, 16, 32, 64]`) confirms the parallel index matches the serial truncation point exactly at every shard count. Behavior is unchanged on purpose — still no resync, only the silence is fixed.
- **Sequencing:** stacked on #3675 and #3695 — merge both first.
---
**Stacked on #3675 (`fix-step-comment-scanning`) and #3695 (`hunt-truncation-observability`) — merge those first.** This PR is based on `fix-step-comment-scanning`'s tip. #3695 fixed the identical defect on the TypeScript side and, in its body, confirmed the Rust side has the same shape and is unfixed. This PR closes that TS/Rust divergence.
## Summary
`EntityScanner::find_entity_end` (`rust/core/src/parser/scanner.rs`) has no byte to resume from once a record opens a `'` string or a `/* ... */` comment that never closes — the inner `memchr::memchr(b'\'', &content[pos..])?` (or `skip_step_comment`) returns `None`, and that `?` used to propagate straight out of `next_entity`, ending the **entire** scan with no trace of why. A truncated download, a failed export, or a lossy round-trip through another tool could silently drop every entity after the break on the WASM/native load path, while the TS fallback (post-#3695) reported it — so which load path a user hit decided whether they knew their model was incomplete.
### RED (executed, pre-fix)
`rust/core/src/parser/scanner_tests.rs::unterminated_string_in_a_record_loses_every_entity_after_it` (temporarily run against the unfixed `scanner.rs` via `git stash`): a fixture with `#11` (valid), `#12` (unterminated string), `#13` (valid) scans to `ids == [11]` — #12 and #13 silently gone, and there was no method on the scanner to say why.
### GREEN (this branch)
`EntityScanner` now exposes `malformed_record_start() -> Option<usize>`, set the first time `find_entity_end` (or the candidate-hunt loop's own `skip_step_comment` call) returns `None`. Behaviour is **unchanged** on purpose: the scan still stops there, no resync — an unterminated string leaves no reliable byte to resume from, and guessing wrong risks fabricating entities from misaligned bytes, the same call #3695 made on the TS side. Only the silence is fixed.
Every whole-file scan across `rust/core`, `rust/processing`, and `rust/wasm-bindings` now reports it through the existing `report_oversized_ids` sink (a new `report_malformed_records`, and the combined `report_scan_diagnostics` convenience both call it):
- `rust/core/src/decoder.rs::build_entity_index`
- `rust/core/src/columnar_index.rs::ColumnarEntityIndex::from_scan`
- `rust/processing/src/processor/mod.rs` (native/server streaming load)
- `rust/wasm-bindings/src/api/parsing.rs` (`scanEntitiesFast`/`scanGeometryEntitiesFast`, the direct WASM twin of TS's `scanIfcEntities`)
- `rust/wasm-bindings/src/api/gpu_meshes/prepass.rs` (`buildPrePassOnce`'s serial/columns path) — also exports a new `malformedRecordFound` boolean on the `entity-index` event, alongside the existing `oversizedIdCount`
## Per-site verdict — every string/comment-state site in the files this hunt named
| Site | Verdict |
|---|---|
| `scanner.rs` `find_entity_end` (the catastrophic one) | **Fixed.** |
| `scanner.rs` `next_entity`'s candidate-hunt loop (`skip_step_comment` between records) | **Fixed**, same shape, same edit. |
| `scanner.rs` `has_non_null_attribute`'s own `in_string` loop (~line 402/481 in the pre-PR file) | **Cannot reach.** Bounded `while pos < content.len()` over an ALREADY-DELIMITED slice `[start, end)` that a prior `find_entity_end` success already terminated — the slice's quotes are balanced by construction. Worst case: the loop just finishes at `content.len()`, mis-reading at most this one attribute — never losing anything past this entity. Same reasoning TS's `entity-extractor.ts` used to clear its own bounded loops. |
| `scanner.rs` `data_section_start` | **Cannot reach.** Bounded by `limit = len.min(1 << 18)`; an unresolved `in_string` at the cap just falls through to the `0` fallback (header-skip heuristic, not entity loss). |
| `tokenizer.rs` `string_literal`/`parse_string_content` | **Cannot reach.** `parse_entity` is only ever called on a single record's already-scanner-validated, already-terminated byte span; an unterminated quote there fails to parse *that one entity* (a `nom::Err`), not the whole scan. |
| `lexical.rs` `skip_step_comment`/`skip_step_trivia` | Pre-existing, deliberate refusal-on-`None` design from #3303 (shared by both fixed call sites above); no change needed to the function itself. |
| #3675's own "unterminated comment inside a record" branch | **Not introduced by #3675** in these Rust files — the comment-refusal-ends-scan shape here predates it, from #3303/#1579. Fixed anyway, by the same one-line-shape edit, since it shares `find_entity_end`'s failure path (see `unterminated_comment_inside_a_record_is_reported`, `unterminated_comment_between_records_is_reported`). |
## The sharded parallel-scan pre-pass — understood before choosing a representation
Per the known blocker: the sibling #3395 oversized-id counter uses `Vec<usize>` of offsets, not a plain counter, because chunk `i > 0` starts speculatively (`EntityScanner::new_at`) and can land inside a quoted value, producing refusals that are artefacts of where the shard started rather than real file content — only the stitch (which knows where a shard's *retained* region begins) can tell them apart.
A malformed-record stop is representable more simply — `Option<usize>`, not a `Vec`, because unlike an oversized-id refusal (skip one record, keep scanning) a malformed stop is **terminal**: `next_entity` never resumes after it, so there is at most one per `EntityScanner` instance. But it needed more than a signal: **byte-identity is this module's whole contract**, and the serial scanner stops *permanently* on a malformed record — nothing past that byte, anywhere in the rest of the file, is ever in the serial index. So the parallel path's stitch (`rust/processing/src/parallel_scan/native.rs::stitch`) now:
- treats a malformed stop inside a chunk's **resynchronised/attributed** region (`chunk.malformed_start >= target`, mirroring the refusal count's `< target` filter) as real, and drops every chunk after it from the merge — same as the existing `expected_start: None` early-`break`;
- treats one inside a chunk's *speculative* prefix (before `target`) as a mid-string artefact and ignores it (the existing `Err`-arm serial rescan already self-heals this case, since a chunk that dies in its own garbage never produces `target` for the binary search to find);
- extends the `Err`-arm's serial rescan (`rescan_range`) to report its own malformed stop too, since that scan starts at a validated real boundary and so is never speculative.
New tests (`rust/processing/src/parallel_scan_tests.rs`) sweep chunk counts `[1, 2, 3, 4, 5, 7, 8, 11, 16, 32, 64]` over a fixture with a malformed record placed so it lands in different shards at different `n`, asserting the parallel index matches the serial truncation point exactly and the stitched flag reports `true` — plus one test confirming the report reaches an installed `set_report_sink`.
**Not extended to the browser's sharded pre-pass** (`prepass_sharded.rs` / `scan_shard_classified_with_refusals`): that path's stitch lives in the host's TypeScript (main thread), not this crate — extending the same attribution there is real, separate TS work, out of scope here. Reported as a gap in the changeset.
## Gates run (foreground, one `cargo` at a time)
- `cargo test -p ifc-lite-core` — 226 lib tests + all integration tests, 0 failed
- `cargo test -p ifc-lite-processing` — including the new `parallel_scan_tests.rs` cases and the pre-existing byte-identity/refusal-parity sweep (unaffected), 0 failed
- `cargo build -p ifc-lite-wasm --target wasm32-unknown-unknown` — clean
- `cargo build --workspace --exclude ifc-lite-wasm` — clean
- `module_size_ratchet` test — clean. `parallel_scan.rs` grew past 400 lines (net new functionality, not just comments) and is split into a sibling `parallel_scan/native.rs`, the same `#[path]` pattern `decoder.rs`/`decoder/caches.rs` already uses (not a `_tests.rs` file, so not ratchet-exempt on that basis — split instead of allowlisted). `scanner.rs`, `processor/mod.rs`, and `prepass.rs` stayed within their existing recorded budgets by trimming doc-comment verbosity, not by touching the allowlist.
Changeset: `@ifc-lite/wasm` patch (`.changeset/report-rust-scanner-malformed-record.md`).
Labelled `unqueued` — this is a hunt-adjacent fix closing a diagnosed TS/Rust divergence, not tied to a `ready` issue.
---
## Merge order (dry-run only — nothing pushed)
Checked against `upstream/main` at `6f445b6f2` (2026-09-03). #3695, #3699, and #3744 all touch the STEP scanner files; dry-run merges (`git merge --no-commit --no-ff` / `git apply --3way` in scratch branches, never pushed) were run in every order that matters, with the merged region read line-by-line afterward rather than trusted on a clean exit.
**TypeScript side (`step-lexing.ts`, `tokenizer.ts`, `scan-worker-source.ts`, `scan-worker-inline.ts`, `entity-scanner.ts`):** #3744 and #3695 auto-merge cleanly in *either* order. Verified after each merge that `isSpaceByte` (step-lexing.ts) and all its inline twins (3× in `tokenizer.ts::scanEntitiesFast`, 1 in `scan-worker-source.ts::skipTriviaAt` + 3 inline copies) still test the full six-byte set (`0x20 0x09 0x0D 0x0A 0x0C 0x0B`), and that `Skip.unterminated?: 'string' | 'comment'` plus its two set-sites in `skipTrivia` are still present. `tsc --noEmit` in `packages/parser` and its vitest suite (93 files / 1039 passed, 2 skipped) are clean in both orders. `entity-refs-from-index.ts` is #3744-only in this set — no overlap.
**Rust side (`scanner.rs`, `scanner_tests.rs`, `lexical.rs`):** `scanner.rs` also auto-merges cleanly with #3699 in either order — verified `malformed_record_start`/`mark_malformed` (the #3699 reporting field, its two call sites, and the accessor) and the three `skip_step_trivia` call sites (the #3744 fix) both survive intact. `scanner_tests.rs` is the one real friction point: **every order produces a git conflict there**, not a silent merge — both PRs append new `#[test]` functions near the same end-of-file anchor. Read both sides: the conflict is purely additive (no test asserts something the other side changes), so resolution is "keep both blocks," not a judgment call. `lexical.rs` is untouched by #3699's own diff, so it's unaffected either way.
1 parent 7659f88 commit afb9725
22 files changed
Lines changed: 947 additions & 328 deletions
File tree
- .changeset
- rust
- core/src
- parser
- processing
- src
- parallel_scan
- processor
- tests
- wasm-bindings/src/api
- gpu_meshes
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
178 | 178 | | |
179 | 179 | | |
180 | 180 | | |
181 | | - | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
182 | 185 | | |
183 | 186 | | |
184 | 187 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| |||
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
39 | | - | |
| 39 | + | |
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
98 | 98 | | |
99 | 99 | | |
100 | 100 | | |
101 | | - | |
102 | | - | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
103 | 104 | | |
104 | 105 | | |
105 | 106 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| 20 | + | |
20 | 21 | | |
| 22 | + | |
21 | 23 | | |
22 | 24 | | |
23 | 25 | | |
24 | 26 | | |
| 27 | + | |
25 | 28 | | |
26 | 29 | | |
27 | 30 | | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
43 | | - | |
| 43 | + | |
44 | 44 | | |
45 | | - | |
46 | | - | |
47 | | - | |
48 | | - | |
49 | | - | |
50 | | - | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
51 | 48 | | |
52 | 49 | | |
53 | 50 | | |
| |||
81 | 78 | | |
82 | 79 | | |
83 | 80 | | |
84 | | - | |
85 | | - | |
86 | | - | |
87 | | - | |
| 81 | + | |
88 | 82 | | |
89 | 83 | | |
90 | 84 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
10 | 14 | | |
11 | 15 | | |
12 | 16 | | |
| |||
20 | 24 | | |
21 | 25 | | |
22 | 26 | | |
| 27 | + | |
| 28 | + | |
23 | 29 | | |
24 | 30 | | |
25 | 31 | | |
| |||
38 | 44 | | |
39 | 45 | | |
40 | 46 | | |
| 47 | + | |
41 | 48 | | |
42 | 49 | | |
43 | 50 | | |
| |||
61 | 68 | | |
62 | 69 | | |
63 | 70 | | |
| 71 | + | |
64 | 72 | | |
65 | 73 | | |
66 | 74 | | |
| |||
99 | 107 | | |
100 | 108 | | |
101 | 109 | | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
102 | 127 | | |
103 | 128 | | |
104 | 129 | | |
| |||
152 | 177 | | |
153 | 178 | | |
154 | 179 | | |
155 | | - | |
156 | | - | |
157 | | - | |
158 | | - | |
159 | | - | |
160 | | - | |
161 | | - | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
162 | 192 | | |
163 | 193 | | |
164 | 194 | | |
| |||
194 | 224 | | |
195 | 225 | | |
196 | 226 | | |
197 | | - | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
198 | 236 | | |
199 | 237 | | |
200 | 238 | | |
| |||
384 | 422 | | |
385 | 423 | | |
386 | 424 | | |
| 425 | + | |
387 | 426 | | |
388 | 427 | | |
389 | 428 | | |
| |||
513 | 552 | | |
514 | 553 | | |
515 | 554 | | |
516 | | - | |
517 | | - | |
518 | | - | |
519 | | - | |
520 | | - | |
521 | | - | |
522 | | - | |
523 | | - | |
524 | | - | |
525 | | - | |
526 | | - | |
527 | | - | |
528 | | - | |
529 | | - | |
530 | | - | |
531 | | - | |
532 | | - | |
533 | | - | |
534 | | - | |
535 | | - | |
536 | | - | |
537 | | - | |
538 | | - | |
539 | | - | |
540 | | - | |
541 | | - | |
542 | | - | |
543 | | - | |
544 | | - | |
545 | | - | |
546 | | - | |
547 | | - | |
548 | | - | |
549 | | - | |
550 | | - | |
551 | | - | |
552 | | - | |
553 | | - | |
554 | | - | |
555 | | - | |
556 | | - | |
557 | | - | |
558 | | - | |
559 | | - | |
560 | | - | |
561 | | - | |
562 | | - | |
563 | | - | |
564 | | - | |
565 | | - | |
566 | | - | |
567 | | - | |
568 | | - | |
569 | 555 | | |
570 | 556 | | |
571 | 557 | | |
0 commit comments