Skip to content

Commit d470d76

Browse files
authored
fix(export): pad the attributes a newer schema appended in the Rust converter (#3271) (#3273)
`convertStepLine` has padded the trailing optional attributes newer schemas ADD since #1416 -- `PredefinedType` on IfcWall / IfcBeam / IfcOpeningElement, IfcMaterial's Description and Category, 61 more. Its Rust port never got the fix, and the Rust port is what `exportStep` runs, so `ifc-lite export --format step --schema IFC4` on an IFC2X3 source wrote entities a positional attribute short -- invalid under the FILE_SCHEMA it declares, which is what #1416 was reported for. The tables were not the problem: the three entity-rename maps and the IFC2X3 attribute-count map diff byte-for-byte against their TypeScript twins over all 3 + 23 + 36 + 30 entries. The missing piece is the algorithm. Padding is only safe where the source schema's positional attribute NAME list is a strict PREFIX of the target's. IfcMaterialProperties goes from [Material] to [Name, Description, Properties, Material]; a trailing `$` there shoves the material reference into the Name slot. Rust has no per-schema attribute table to test that against -- the generated `IfcType` is IFC4X3 alone -- so `schema_pad` carries the strict-prefix pairs derived from the same generated buildingSMART tables the TypeScript side reads at run time. Neither half is trusted to have stayed in step with the other. Both now run their own converter over one shared fixture (`rust/export/tests/fixtures/schema_upconvert_sweep.json`), following the `rooted_type_parity.rs` precedent. WHICH rows that fixture must hold is re-derived from `schema_pad`'s own tables rather than floored by a count: a count survives dropping the rows that matter, and stays silent on a type added to the tables with no row to pin it. A negative control covers the direction the positive rows cannot -- deleting the strict-prefix restriction and padding every short line passes all 134 padded rows and fails `a_reordered_attribute_list_is_never_padded`. `schema_convert.rs`'s inline test module moves to `schema_convert_tests.rs` via the `#[path]` convention (`schema_helpers.rs`, `georef.rs`) so the module stays under the 400-line ratchet. Refs #3271
1 parent b342063 commit d470d76

8 files changed

Lines changed: 1626 additions & 135 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
"@ifc-lite/wasm": patch
3+
---
4+
5+
Pad the attributes a newer schema appended when the Rust STEP converter upgrades a file.
6+
7+
`convertStepLine` in `packages/export` has padded the trailing optional attributes newer schemas ADD since #1416`PredefinedType` on `IfcWall` / `IfcBeam` / `IfcOpeningElement`, `IfcMaterial`'s `Description` and `Category`, and 61 more. Its Rust port never got the fix, and the Rust port is what `exportStep` runs, so `ifc-lite export --format step --schema IFC4` on an IFC2X3 source wrote entities one or more positional attributes short. That is an invalid IFC4 file, and strict readers reject it. Verbatim, before:
8+
9+
```
10+
#1=IFCWALL('0aBcDeFgHiJkLmNoPqRsTu',$,'W1',$,$,$,$,'tag');
11+
```
12+
13+
and after:
14+
15+
```
16+
#1=IFCWALL('0aBcDeFgHiJkLmNoPqRsTu',$,'W1',$,$,$,$,'tag',$);
17+
```
18+
19+
Padding applies only where the source schema's positional attribute NAME list is a strict PREFIX of the target's — the same restriction the TypeScript half enforces at run time. Entities that reorder or insert mid-list (`IfcMaterialProperties` goes from `[Material]` to `[Name, Description, Properties, Material]`) are left untouched, because a trailing `$` there would shove existing values into the wrong and type-invalid slots.
20+
21+
The two implementations are now pinned to one shared fixture, `rust/export/tests/fixtures/schema_upconvert_sweep.json`, whose rows are derived from the generated buildingSMART attribute tables and which NAMES every padded type rather than counting them — a count floor stays silent exactly when a row is dropped.
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
4+
5+
/**
6+
* TypeScript half of the schema-upconversion padding parity pin.
7+
*
8+
* `convertStepLine` pads the trailing optional attributes a newer target
9+
* schema APPENDED (#1416). The Rust port in
10+
* `rust/export/src/schema_convert.rs` did not — and it is the Rust one that
11+
* `ifc-lite export --format step --schema IFC4` runs, through `exportStep` in
12+
* the wasm bindings — so the two halves emitted different files for the same
13+
* conversion for two months. Both are now held to ONE fixture; the Rust half
14+
* is `rust/export/tests/schema_upconvert_parity.rs`. Follows the
15+
* `rooted-type-sweep.parity.test.ts` precedent.
16+
*
17+
* The fixture's padded rows are DERIVED from
18+
* `packages/data/src/ifc-schema/generated/entities-*.ts` (the types whose
19+
* source attribute NAME list is a strict prefix of the target's — the same
20+
* relation `isStrictAttrPrefix` tests at run time), so this side is not
21+
* circular: it pins the answer that generated data implies, and a regenerated
22+
* schema that changes a count fails here rather than silently changing what
23+
* `ifc-lite` writes.
24+
*
25+
* WHICH rows the fixture must contain is pinned on the Rust side only
26+
* (`fixture_names_every_padded_type`), which re-derives the universe from its
27+
* own tables. The count check below is a smoke test, not the coverage gate.
28+
*/
29+
import { describe, it, expect } from 'vitest';
30+
import { readFileSync } from 'node:fs';
31+
import { fileURLToPath } from 'node:url';
32+
import { convertStepLine, type IfcSchemaVersion } from './schema-converter.js';
33+
34+
interface SweepCase {
35+
why: string;
36+
from: IfcSchemaVersion;
37+
to: IfcSchemaVersion;
38+
line: string;
39+
expect: string;
40+
}
41+
42+
// The fixture lives in the Rust crate so `include_str!` can reach it; this side
43+
// resolves it relative to the source file. NOT guarded by `existsSync`: a
44+
// missing fixture means the pin is not being enforced, which must fail loudly.
45+
const fixturePath = fileURLToPath(
46+
new URL('../../../rust/export/tests/fixtures/schema_upconvert_sweep.json', import.meta.url),
47+
);
48+
const fixture: { cases: SweepCase[] } = JSON.parse(readFileSync(fixturePath, 'utf8'));
49+
50+
describe('convertStepLine matches the shared cross-language upconversion sweep', () => {
51+
it('the fixture is not empty or trivial', () => {
52+
expect(fixture.cases.length).toBeGreaterThan(100);
53+
});
54+
55+
it('every case agrees with the Rust converter', () => {
56+
const mismatches = fixture.cases
57+
.map((c) => ({ c, got: convertStepLine(c.line, c.from, c.to) }))
58+
.filter(({ c, got }) => got !== c.expect)
59+
.map(({ c, got }) => `${c.from}->${c.to} [${c.why}]\n in ${c.line}\n expect ${c.expect}\n got ${got}`);
60+
expect(mismatches).toEqual([]);
61+
});
62+
63+
it('a reordered attribute list is never padded (negative control)', () => {
64+
// IFC2X3 IfcMaterialProperties is [Material]; IFC4 is
65+
// [Name, Description, Properties, Material]. Padding would read #8 as Name.
66+
expect(convertStepLine('#7=IFCMATERIALPROPERTIES(#8);', 'IFC2X3', 'IFC4')).toBe(
67+
'#7=IFCMATERIALPROPERTIES(#8);',
68+
);
69+
});
70+
});

rust/export/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ mod parquet_bos;
3939
mod rooms;
4040
pub mod rooted_type;
4141
mod schema_convert;
42+
mod schema_pad;
43+
pub use schema_pad::padded_type_universe;
4244
mod shades;
4345
mod step;
4446
mod step_cow;

rust/export/src/schema_convert.rs

Lines changed: 23 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
//! IFC **schema conversion** for STEP export (Phase 2 P2). Ports
33
//! `packages/export/src/schema-converter.ts`: entity-type renames between
44
//! IFC2X3 / IFC4 / IFC4X3 / IFC5 (with multi-step chaining), IFC2X3 attribute-count
5-
//! trimming on downgrade, and a proxy fallback for types with no target representation.
5+
//! trimming on downgrade, padding of the attributes a newer target schema appended
6+
//! (`schema_pad`), and a proxy fallback for types with no target representation.
67
78
/// Canonicalize a FILE_SCHEMA label to one of the four families we convert between.
89
fn canon(s: &str) -> &'static str {
@@ -232,7 +233,7 @@ pub fn convert_step_line(line: &str, from: &str, to: &str, express_id: u32) -> S
232233
);
233234
}
234235

235-
let final_attrs = if cto == "IFC2X3" {
236+
let mut final_attrs = if cto == "IFC2X3" {
236237
match ifc2x3_attr_count(&new_type) {
237238
Some(max) => trim_attributes(attrs, max),
238239
None => attrs.to_string(),
@@ -241,6 +242,24 @@ pub fn convert_step_line(line: &str, from: &str, to: &str, express_id: u32) -> S
241242
attrs.to_string()
242243
};
243244

245+
// Pad the trailing optional attributes a newer target schema APPENDED
246+
// (#1416). Keyed on the ORIGINAL type, because the source schema is what
247+
// decides how many attributes the line already has; the table's target
248+
// count already accounts for the rename. Only types whose source
249+
// attribute NAME list is a strict prefix of the target's are in it, so
250+
// this can never shift a value into a reordered slot -- see
251+
// `schema_pad`. An attribute-less line is left alone rather than
252+
// fabricated from nothing, matching the TypeScript twin's
253+
// `currentCount > 0` guard.
254+
if let Some(target_count) = crate::schema_pad::padded_attr_count(cfrom, cto, &entity_type) {
255+
let current = crate::schema_pad::count_top_level_attributes(&final_attrs);
256+
if current > 0 && current < target_count {
257+
for _ in current..target_count {
258+
final_attrs.push_str(",$");
259+
}
260+
}
261+
}
262+
244263
format!("{prefix}{new_type}({final_attrs});")
245264
}
246265

@@ -250,136 +269,5 @@ pub fn needs_conversion(from: &str, to: &str) -> bool {
250269
}
251270

252271
#[cfg(test)]
253-
mod tests {
254-
use super::*;
255-
256-
#[test]
257-
fn entity_type_renames() {
258-
assert_eq!(convert_entity_type("IFCBURNERTYPE", "IFC4", "IFC2X3"), "IFCGASTERMINALTYPE");
259-
assert_eq!(convert_entity_type("IFCCHIMNEY", "IFC4", "IFC2X3"), "IFCBUILDINGELEMENTPROXY");
260-
assert_eq!(convert_entity_type("IFCWALL", "IFC2X3", "IFC4"), "IFCWALL"); // unchanged
261-
// chained 4X3 → 2X3 (via 4): IfcFacility → IfcBuilding
262-
assert_eq!(convert_entity_type("IFCFACILITY", "IFC4X3", "IFC2X3"), "IFCBUILDING");
263-
}
264-
265-
/// The UPGRADE table (`map_2x3_to_4`) had no test in this crate: the only
266-
/// 2X3 → 4 case above is `IFCWALL`, a type that is unchanged in every
267-
/// schema, so replacing the whole arm with a pass-through left the entire
268-
/// `ifc-lite-export` suite green (confirmed by mutation). The TS twin
269-
/// `packages/export/src/schema-converter.test.ts` covers this direction;
270-
/// the Rust port is what the CLI, server and wasm actually run.
271-
///
272-
/// Concretely, un-renamed output is an INVALID file: none of these three
273-
/// type names exists in IFC4.
274-
#[test]
275-
fn ifc2x3_only_types_are_renamed_on_upgrade() {
276-
for (from, want) in [
277-
("IFCELECTRICDISTRIBUTIONPOINT", "IFCELECTRICDISTRIBUTIONBOARD"),
278-
("IFCGASTERMINALTYPE", "IFCBURNERTYPE"),
279-
("IFCEQUIPMENTELEMENT", "IFCBUILDINGELEMENTPROXY"),
280-
] {
281-
assert_eq!(convert_entity_type(from, "IFC2X3", "IFC4"), want, "2X3 → 4");
282-
// 2X3 → 4X3 and 2X3 → 5 route through the same table.
283-
assert_eq!(convert_entity_type(from, "IFC2X3", "IFC4X3"), want, "2X3 → 4X3");
284-
assert_eq!(convert_entity_type(from, "IFC2X3", "IFC5"), want, "2X3 → 5");
285-
}
286-
}
287-
288-
/// The direct `4X3 → 4` arm was only ever reached by types it does NOT
289-
/// rename: `entity_type_renames` chains through it on the way to 2X3, and
290-
/// `alignment_becomes_proxy_on_downgrade` hits the proxy branch instead.
291-
/// Replacing the arm with a pass-through likewise left the suite green.
292-
/// `IFC5 → IFC4` shares the arm, so it is pinned here too — nothing else
293-
/// in this crate exercises `canon`'s IFC5 branch at all.
294-
#[test]
295-
fn ifc4x3_and_ifc5_types_are_renamed_down_to_ifc4() {
296-
for (from, want) in [
297-
("IFCBRIDGE", "IFCBUILDING"),
298-
("IFCBRIDGEPART", "IFCBUILDINGSTOREY"),
299-
("IFCPAVEMENT", "IFCSLAB"),
300-
("IFCCAISSONFOUNDATION", "IFCFOOTING"),
301-
("IFCDISTRIBUTIONBOARD", "IFCELECTRICDISTRIBUTIONBOARD"),
302-
] {
303-
assert_eq!(convert_entity_type(from, "IFC4X3", "IFC4"), want, "4X3 → 4");
304-
assert_eq!(convert_entity_type(from, "IFC5", "IFC4"), want, "5 → 4");
305-
}
306-
// 4 ↔ 4X3 ↔ 5 carry no renames, and IFCX* canonicalizes to IFC5.
307-
assert_eq!(convert_entity_type("IFCWALL", "IFC4", "IFC5"), "IFCWALL");
308-
assert_eq!(convert_entity_type("IFCBRIDGE", "IFCX", "IFC4"), "IFCBUILDING");
309-
assert!(!needs_conversion("IFC5", "IFCX"));
310-
}
311-
312-
#[test]
313-
fn downgrade_trims_attributes() {
314-
// IfcWall in IFC4 has 9 attrs (trailing PredefinedType); IFC2X3 keeps 8.
315-
let line = "#5=IFCWALL('guid',$,'W1',$,$,#6,#7,'tag',.STANDARD.);";
316-
let out = convert_step_line(line, "IFC4", "IFC2X3", 5);
317-
assert!(out.starts_with("#5=IFCWALL("), "type kept");
318-
assert!(!out.contains(".STANDARD."), "9th attr (PredefinedType) trimmed");
319-
// 8 top-level attrs remain → 7 commas.
320-
let inner = &out["#5=IFCWALL(".len()..out.len() - 2];
321-
assert_eq!(inner.split(',').count(), 8, "trimmed to 8 attrs");
322-
}
323-
324-
#[test]
325-
fn nested_attrs_not_split_when_trimming() {
326-
// Commas inside a nested list must not count as top-level separators.
327-
let line = "#9=IFCWALL('g',$,$,$,$,(#1,#2,#3),#7,'t',.STANDARD.);";
328-
let out = convert_step_line(line, "IFC4", "IFC2X3", 9);
329-
assert!(out.contains("(#1,#2,#3)"), "nested list preserved intact");
330-
assert!(!out.contains(".STANDARD."), "trailing attr trimmed");
331-
}
332-
333-
#[test]
334-
fn alignment_becomes_proxy_on_downgrade() {
335-
let line = "#3=IFCALIGNMENTHORIZONTAL('g',$,$,$,$,#4);";
336-
let out = convert_step_line(line, "IFC4X3", "IFC4", 3);
337-
assert!(out.starts_with("#3=IFCPROXY("), "alignment → proxy");
338-
assert!(out.contains("'IFCALIGNMENTHORIZONTAL'"), "original type recorded as name");
339-
}
340-
341-
/// PINS the schema-downgrade proxy-GlobalId divergence between the two
342-
/// exporters (#3015) AS divergence -- it does not fix it. Which side wins
343-
/// is a maintainer decision, not something a test should resolve
344-
/// unilaterally.
345-
///
346-
/// `placeholder_guid` here derives the id purely from the express id
347-
/// (`id as u64 + 0x1000_0000`, base64-stamped). The TypeScript twin
348-
/// (`convertStepLine` in `packages/export/src/schema-converter.ts`)
349-
/// derives it from `deterministicGlobalId` of the WHOLE source line
350-
/// (`ifcproxy:{prefix}{entityType}({attrs})`) -- a different algorithm
351-
/// entirely, not just a different seed to the same one. Verified by
352-
/// actually running both on the byte-identical input line below;
353-
/// `packages/export/src/schema-converter.test.ts`'s
354-
/// `placeholder_guid_diverges_from_the_rust_mint_pinned_not_fixed` pins
355-
/// the TS side of the same pair.
356-
///
357-
/// If this test ever starts failing because the values converged, that
358-
/// is good news -- update the doc here (and the TS twin) to say so,
359-
/// don't just delete the assertion.
360-
#[test]
361-
fn placeholder_guid_diverges_from_the_typescript_mint_pinned_not_fixed() {
362-
let line = "#42=IFCALIGNMENTSEGMENT('2K5H1$Zs9CQuKQFQKQFQKQ',#1,'A',$,$,#7,#9,$);";
363-
let out = convert_step_line(line, "IFC4X3", "IFC4", 42);
364-
let guid = out.split('\'').nth(1).expect("IFCPROXY line has a quoted GlobalId");
365-
assert_eq!(
366-
guid, "00000000000000000G000g",
367-
"Rust's placeholder_guid(42) output changed -- update this pin (and check whether \
368-
it now agrees with the TS twin, in which case update both docs to say so)"
369-
);
370-
assert_ne!(
371-
guid, "3m5OyAyREn46dEymqijDwc",
372-
"this is the TS side's minted value for the byte-identical input line -- if Rust \
373-
now matches it, the divergence has been resolved; update both tests' docs instead \
374-
of silently dropping this assertion"
375-
);
376-
}
377-
378-
#[test]
379-
fn no_conversion_is_identity() {
380-
let line = "#1=IFCWALL('g',$,$);";
381-
assert_eq!(convert_step_line(line, "IFC4", "IFC4", 1), line);
382-
assert!(!needs_conversion("IFC4", "IFC4"));
383-
assert!(needs_conversion("IFC2X3", "IFC4"));
384-
}
385-
}
272+
#[path = "schema_convert_tests.rs"]
273+
mod tests;

0 commit comments

Comments
 (0)