fix(parser): the last-resort schema scan folded Unicode, so ıFC5 selected IFC5 (#3284) (#3315)

* Fold ASCII in the last-resort schema scan, and make its tests able to fail

Follow-up to #3297, which merged at the head I had pushed rather than the one
I had finished. Three commits did not make it, and one of them is a real fix
rather than polish, so this lands them.

THE FIX. `detectSchemaVersion`'s fallback uppercases the first 2000 bytes and
looks for `IFC5` / `IFC4X3` / `IFC4` / `IFC2X3` as substrings.
`'ı'.toUpperCase()` is `'I'`, so a FILE_DESCRIPTION mentioning `ıFC5` selects
IFC5 for a file that never said so. Still live on main at source-header.ts:294.

Same fold #3297 removed from the record scan, one function further down the
same file. The scan is deliberately loose -- it only runs when no FILE_SCHEMA
identifier resolves, and it already matches `IFC4` inside ordinary prose --
but loose is not a reason to accept a fold ISO 10303-21 does not use. A copy
is fine here where it was not in the record scan, because nothing takes
offsets from it.

THE TESTS THAT COULD NOT FAIL. My first two tests for this did not exercise
the fold at all: one asserted the trailing IFC4 default, which passes for any
implementation that fails to match, and the other fed input already
upper-case. An identity mutant on the helper was killed by ZERO tests across
the whole parser suite. There is now a case that drives the direction the fold
exists for, lower-case `ifc4x3` in prose, plus one for the subtler mutant that
DROPS non-ASCII rather than passing it through: deleting a character joins the
fragments either side, so `IFCı5` becomes `IFC5`, a match built from a
character that was never in the word.

Off-by-one bounds on the fold survive and are left alone deliberately. The
output is consumed only by `.includes()` on tokens whose letters are i, f, c
and x, so neither `a` nor `z` can appear in a match and nothing through the
public surface can distinguish them.

A FALSE CLAIM, replacing a stale one. #3297 rewrote a comment in
`schema-version-detection.test.ts` that wrongly said `detectSchemaVersion` is
module-private, and replaced it with a different wrong claim: that
`buildStep()` can never reach the last-resort scan. It always emits a
FILE_SCHEMA record but not always a RESOLVABLE one, and the `IFC2X2` case
falls through to the scan. Proven by putting a throw at the top of the scan
and watching only that test go red.

Also: `schema_detect.rs` uses the crate's SPDX one-line header like every
sibling, both changeset fences declare a language, and the changeset says the
`ıFC5` input falls through to the IFC4 default rather than "no longer selects
a schema", since `detectSchemaVersion` always returns one.

Verified by exit code: parser 849, rust export 0, typecheck 0, lint 0,
module-size 0. Mutation-verified: restoring `toUpperCase()` reddens the new
test and only it.

* Drop the license-header change, and say what the fold gives up

Preflight came back clean on the fix itself and raised two small things.

The SPDX header swap on `schema_detect.rs` has nothing to do with the ASCII
fold, so it is out. It was a CodeRabbit suggestion I took on the original
branch, and it is defensible -- 52 of 54 files in `rust/export/src` already
use the one-line form -- but `LICENSE_HEADER.md` still documents the block
comment as required for `.rs`, and `scripts/add-license-headers.mjs` matches
only that form. So the repo has an in-flight migration with a stale doc and a
stale script, and quietly adding one more file to the wrong side of it in a
parser fix is not the way to settle that. Filing it separately.

The changeset now says what the fold costs rather than only what it fixes: a
Turkish-locale `ıfc4x3` in free header prose used to resolve and no longer
does. It is the same character as the false positive being removed, pointed
the other way, and a reader of release notes should see both. ISO 10303-21
tokens are ASCII and this scan only runs for a file that declares no
resolvable schema, so the trade is worth making, but it is a trade.

Also `source-header.test.ts`'s own docstring claimed the file is direct
coverage for `parseSourceHeader`. It now tests `detectSchemaVersion` too, and
the sibling comment in `schema-version-detection.test.ts` -- rewritten in this
same work -- points at it for exactly that. The two now agree.

Verified by exit code: parser 849, typecheck 0, lint 0, module-size 0.

* Two corrections to what #3297 shipped, both found by the CLI after it merged

Neither blocked that merge; both are mine.

A DOC THAT SURVIVED ITS OWN MECHANISM. `find_unquoted`'s comment sends the
reader to `last_comment_close` for the linearity argument and describes the
closer search as HOISTED. I replaced that design mid-branch with a deferred
search and a `no_closer` memo on `Lex`, and deleted the function, but the
comment two files away still described the old shape. `grep -rn "fn
last_comment_close" rust/export/src/` returns nothing.

That is exactly the failure #3284 is about, committed by the fix for it: a
comment invalidated at a distance by a refactor, still confidently describing
a mechanism that no longer exists. Now it names the memo and says why the memo
is what makes the bound hold.

AN ASSERTION THAT COULD NOT TELL TWO ANSWERS APART.

    assert!(h.is_none() || h.unwrap().schema_identifiers.is_empty());

passes whether the reader REJECTS the malformed `FILE_SCHEMA\u{00A0}(...)` or
ACCEPTS it and returns an empty list. The comment directly above claims the
first. So the test agreed with itself either way, and if the reader ever began
accepting that record it would still be green.

It returns None today, so that is what is pinned now. Mutation-verified rather
than assumed: teaching `skip_trivia` to treat 0xC2, the UTF-8 lead byte of
U+00A0, as whitespace makes the reader accept the record, and the tightened
assertion reddens where the old one did not.

This also gives the PR a new head event, which it needs for a second reason:
`gh run list --branch fix/3284-followup-ascii-fold` returned NOTHING, so
test.yml never fired when the branch was pushed and the PR opened. Seven lanes
registered, none of them a test lane, and the required aggregate absent. Same
shape as #3294, on my own PR, which is what #3313 exists to catch.

Verified by exit code: cargo test -p ifc-lite-export 0.

Author: louistrue

.changeset/quote-aware-header-scan.md

@@ -9,15 +9,15 @@ Three things were wrong, and each lost more than the comment it came from. An ap
 
 On the Rust side the cost is the exported file, because `export_step` falls back to its own defaults whenever `parse_source_header` returns nothing. One comment in a header was enough to turn this:
 
-```
+```text
 FILE_DESCRIPTION(('ViewDefinition [CoordinationView_V2.0]'),'2;1');
 FILE_NAME('export.ifc','2024-01-01T00:00:00',('Ann'),('Acme Ltd'),'ifc-lite','TheirSystem','contract-77');
 FILE_SCHEMA(('IFC4X3'));
 ```
 
 into this:
 
-```
+```text
 FILE_DESCRIPTION(('Exported from ifc-lite'),'2;1');
 FILE_NAME('export.ifc','',(''),(''),'ifc-lite','ifc-lite','');
 FILE_SCHEMA(('IFC4'));
@@ -30,3 +30,7 @@ Author, organization, authorization and time stamp are emptied, the description
 Keyword matching in the TypeScript reader now folds ASCII case per character rather than uppercasing a copy of the text. Indexing a copy shifted every offset after a value whose uppercase is longer, so a header describing `Straße` lost its entire `FILE_NAME` record, and a full Unicode fold read an unquoted `ENDſEC` as `ENDSEC` and truncated the header there. The Rust reader already folded per byte.
 
 One behaviour is now stricter, in the TypeScript reader only. Whitespace between a record keyword and its `(` is ASCII, which is what ISO 10303-21 means, where it previously accepted any Unicode space separator. A header written with `U+00A0` there resolved before and does not now. That is the answer the Rust half already gave, so the two agree rather than one being widened to match the other.
+
+The last-resort schema scan folds ASCII too, for the same reason. It only runs when no `FILE_SCHEMA` identifier resolves at all, and it uppercased the first 2000 bytes before looking for `IFC5` / `IFC4X3` / `IFC4` / `IFC2X3` as substrings. `ı` uppercases to `I`, so a description mentioning `ıFC5` selected IFC5 for a file that never said so. That input now falls through to the IFC4 default instead. Lower-case prose still resolves.
+
+The one thing this gives up is a Turkish-locale lowercasing: `ıfc4x3` written in free header prose used to resolve to IFC4X3 and now does not. ISO 10303-21 tokens are ASCII, and this scan only runs for a file that declares no resolvable schema at all, so the trade is one exotic spelling against the false positives above.

packages/parser/src/source-header.ts

@@ -259,6 +259,16 @@ function schemaFromIdentifier(identifier: string): IfcStoreBase['schemaVersion']
   return undefined;
 }
 
+/** Upper-case the ASCII letters and nothing else. See `detectSchemaVersion`. */
+function asciiUpper(text: string): string {
+  let out = '';
+  for (let i = 0; i < text.length; i++) {
+    const c = text.charCodeAt(i);
+    out += c >= 97 && c <= 122 ? String.fromCharCode(c - 32) : text[i];
+  }
+  return out;
+}
+
 /**
  * Determine which IFC schema a STEP buffer declares (issue #3278).
  *
@@ -291,7 +301,14 @@ export function detectSchemaVersion(
 
   const src = asSourceBytes(buffer);
   const headerEnd = Math.min(src.byteLength, 2000);
-  const headerText = src.decodeUtf8(0, headerEnd).toUpperCase();
+  // ASCII-only, for the same reason `matchesKeywordAt` is. `toUpperCase()`
+  // maps `ı` (dotless i) to `I`, so a FILE_DESCRIPTION mentioning `ıFC5` chose
+  // IFC5 for a file that never said so. This scan is already a loose
+  // last-resort substring match -- it only runs when no FILE_SCHEMA identifier
+  // resolved at all -- but loose is not a reason to accept a fold 10303-21
+  // does not use. Offsets are not taken from this copy, so a copy is fine here
+  // where it was not in the record scan.
+  const headerText = asciiUpper(src.decodeUtf8(0, headerEnd));
 
   if (headerText.includes('IFC5')) return 'IFC5';
   if (headerText.includes('IFC4X3')) return 'IFC4X3';

packages/parser/test/schema-version-detection.test.ts

@@ -3,7 +3,7 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 /**
- * Coverage for `detectSchemaVersion()` (columnar-parser.ts), which reads the
+ * Coverage for `detectSchemaVersion()` (source-header.ts), which reads the
  * `FILE_SCHEMA` token out of the STEP header and classifies it via an
  * ordered ladder of `.includes()` checks:
  *
@@ -22,10 +22,17 @@
  * entity-dictionary tables, not header detection. Swapping the IFC4X3/IFC4
  * lines survives the full test suite before this file.
  *
- * `detectSchemaVersion` is module-private; this exercises it exclusively
- * through the public `ColumnarParser.parseLite()` entry point rather than
- * exporting it, since the whole point of the ladder is what schemaVersion
- * a real caller observes on the resulting store.
+ * This file drives `detectSchemaVersion` exclusively through the public
+ * `ColumnarParser.parseLite()` entry point, because the whole point of the
+ * ladder is what schemaVersion a real caller observes on the resulting store.
+ * That is a choice, not a constraint: the function is exported, and
+ * `source-header.test.ts` calls it directly.
+ *
+ * `buildStep()` always emits a FILE_SCHEMA record, but not always a RESOLVABLE
+ * one: the `IFC2X2` case below declares a token no prefix matches, so it falls
+ * through the identifier loop and its `IFC4` answer comes from the last-resort
+ * scan's trailing default rather than from the declaration. That case is
+ * therefore sensitive to changes in the scan, which the others are not.
  */
 
 import { describe, it, expect } from 'vitest';

packages/parser/test/source-header.test.ts

@@ -3,7 +3,9 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 /**
- * Direct coverage for {@link parseSourceHeader}.
+ * Direct coverage for {@link parseSourceHeader}, and for
+ * {@link detectSchemaVersion}'s last-resort scan, which the ladder in
+ * `schema-version-detection.test.ts` reaches only through its `IFC2X2` case.
  *
  * The header is read on every parse (`columnar-parser`) and written back out on
  * every STEP export (`step-exporter`), so a decoding error here silently
@@ -17,7 +19,7 @@
  */
 
 import { describe, expect, it } from 'vitest';
-import { parseSourceHeader } from '../src/source-header.js';
+import { detectSchemaVersion, parseSourceHeader } from '../src/source-header.js';
 import { StepTextScan } from '../src/step-lexing.js';
 import { contiguousSourceBytes, type IfcSourceBytes } from '../src/source-bytes.js';
 
@@ -534,3 +536,50 @@ describe('the comment scan stays linear on hostile input (#3284)', () => {
     expect(scan.searches).toBe(500);
   });
 });
+
+describe('the last-resort schema scan folds ASCII only (#3284)', () => {
+  it('a dotless i in header prose does not select a schema', () => {
+    // `'ı'.toUpperCase()` is `'I'`, so uppercasing the whole header made
+    // `ıFC5` in a description read as `IFC5`. This scan only runs when no
+    // FILE_SCHEMA identifier resolved, and it is already a loose substring
+    // match, but a fold ISO 10303-21 does not use is not one of the ways it is
+    // allowed to be loose.
+    const before =
+      'ISO-10303-21;\nHEADER;\n' +
+      "FILE_DESCRIPTION(('exported by ıFC5 Studio'),'2;1');\n" +
+      'ENDSEC;\nDATA;\nENDSEC;\n';
+    expect(detectSchemaVersion(enc(before), parseSourceHeader(enc(before)))).toBe('IFC4');
+
+    // And INSIDE the token, which is the harder direction. Dropping a
+    // non-ASCII character instead of passing it through joins the fragments
+    // either side of it, so `IFCı5` would become `IFC5` -- a false match built
+    // out of a character that was never part of the word. Folding correctly
+    // and deleting incorrectly agree on the case above and disagree here.
+    const inside =
+      'ISO-10303-21;\nHEADER;\n' +
+      "FILE_DESCRIPTION(('exported by IFCı5 Studio'),'2;1');\n" +
+      'ENDSEC;\nDATA;\nENDSEC;\n';
+    expect(detectSchemaVersion(enc(inside), parseSourceHeader(enc(inside)))).toBe('IFC4');
+  });
+
+  it('lower-case prose still resolves, which is what the fold is FOR', () => {
+    // The other two cases here pass even if `asciiUpper` folds nothing: one
+    // asserts the IFC4 default, and the other feeds input that is already
+    // upper-case. This is the one that dies if the fold stops folding, and it
+    // is the direction the doc comment promises -- every file that resolved
+    // before keeps resolving the same way.
+    const text =
+      'ISO-10303-21;\nHEADER;\n' +
+      "FILE_DESCRIPTION(('exported by acme ifc4x3 exporter'),'2;1');\n" +
+      'ENDSEC;\nDATA;\nENDSEC;\n';
+    expect(detectSchemaVersion(enc(text), parseSourceHeader(enc(text)))).toBe('IFC4X3');
+  });
+
+  it('still finds a real identifier in the same position', () => {
+    const text =
+      'ISO-10303-21;\nHEADER;\n' +
+      "FILE_DESCRIPTION(('exported by IFC5 Studio'),'2;1');\n" +
+      'ENDSEC;\nDATA;\nENDSEC;\n';
+    expect(detectSchemaVersion(enc(text), parseSourceHeader(enc(text)))).toBe('IFC5');
+  });
+});

rust/export/src/schema_detect.rs

@@ -35,10 +35,12 @@ use crate::source_header::Lex;
 /// it handles the `''` escape by consuming a literal whole instead of toggling
 /// a flag per apostrophe.
 ///
-/// Linear in `haystack.len()`. That is not free here and the reason is in
-/// `last_comment_close`: hoisting the closer search is what keeps an
-/// unterminated `/*` from making this quadratic, and this function has no
-/// header cap, so it can be handed a whole multi-megabyte file.
+/// Linear in `haystack.len()`, and that is not free here. An unterminated
+/// `/*` makes the naive form quadratic, and this function has no header cap,
+/// so it can be handed a whole multi-megabyte file. What prevents it is the
+/// `no_closer` memo on `source_header::Lex`: the closer search is deferred
+/// until a `/*` is actually seen, and one failure proves no later `/*` can
+/// open a comment either.
 fn find_unquoted(haystack: &[u8], needle: &[u8]) -> Option<usize> {
     if needle.is_empty() || needle.len() > haystack.len() {
         return None;

rust/export/tests/source_header_comments.rs

@@ -111,7 +111,11 @@ fn the_accepted_whitespace_set_is_exactly_the_ascii_one() {
 
     // And nothing outside it. U+00A0 is not whitespace in ISO 10303-21, so the
     // record is malformed and both halves decline it.
+    //
+    // Asserted as `is_none()` rather than "none OR an empty list". The looser
+    // form passed whether the reader REJECTED the record or ACCEPTED it and
+    // came back with nothing, so it could not tell those apart, and the
+    // comment above it claims the first. It returns None today; pin that.
     let nbsp = header("FILE_SCHEMA\u{00A0}(('IFC2X3'));");
-    let h = parse_source_header(&nbsp);
-    assert!(h.is_none() || h.unwrap().schema_identifiers.is_empty());
+    assert!(parse_source_header(&nbsp).is_none());
 }