From 4ab1071162bced89140ad59fbf783d79ba31637b Mon Sep 17 00:00:00 2001 From: Orual Date: Mon, 10 Aug 2026 20:01:19 -0400 Subject: [PATCH] PM-86: fix compat harness for removed ModelSummary.semantic Rebuild LdMeshSemantic from OwnedParseResult.geometry instead of the deleted ModelSummary.semantic field. Epic: PM-86 --- crates/polymodel-ldraw-core/src/adapter.rs | 69 -------- crates/polymodel-ldraw-core/src/lib.rs | 180 ++++++++++++++++----- crates/polymodel-ldraw-core/src/model.rs | 4 - crates/polymodel-ldraw-core/src/parser.rs | 17 -- tools/ldraw-compat-harness/src/main.rs | 111 +++++++++++-- 5 files changed, 238 insertions(+), 143 deletions(-) diff --git a/crates/polymodel-ldraw-core/src/adapter.rs b/crates/polymodel-ldraw-core/src/adapter.rs index 4f73062..84e6681 100644 --- a/crates/polymodel-ldraw-core/src/adapter.rs +++ b/crates/polymodel-ldraw-core/src/adapter.rs @@ -3,49 +3,10 @@ use crate::{ ParseOptions, ParserProfile, RootId, }; use std::borrow::Cow; -use std::collections::HashMap; - /// Host-facing adapter: host code performs discovery/fetching and supplies the /// complete immutable materialization table to the synchronous core. pub struct InProcessRustAdapter; -#[derive(Default)] -pub(crate) struct CompatibilitySink { - pub(crate) geometry: HashMap>, -} - -impl crate::SemanticSink for CompatibilitySink { - fn on_event(&mut self, event: &crate::SemanticEvent<'_>, _context: &crate::TraversalContext) { - let crate::SemanticEvent::GeometryEmitted { - primitive_id, - source, - span, - line_type, - colour, - vertices, - bfc, - texmap, - inverted, - } = event - else { - return; - }; - self.geometry - .entry(source.clone()) - .or_default() - .push(crate::GeometryRecord { - primitive_id: *primitive_id, - source: source.clone(), - span: *span, - line_type: *line_type, - colour: *colour, - vertices: vertices.to_vec(), - bfc: *bfc, - texmap: texmap.cloned(), - inverted: *inverted, - }); - } -} #[derive(Clone)] pub struct AdapterRequest<'src> { pub bytes: &'src [u8], @@ -58,20 +19,6 @@ pub struct AdapterRequest<'src> { pub cancellation: CancellationPolicy, } impl InProcessRustAdapter { - pub fn parse<'src>(request: AdapterRequest<'src>) -> Result { - Self::parse_with_limits(request, LdrawLimits::default()) - } - - pub fn parse_with_limits<'src>( - request: AdapterRequest<'src>, - limits: LdrawLimits, - ) -> Result { - let mut sink = CompatibilitySink::default(); - let mut result = Self::parse_with_limits_and_sink(request, limits, &mut sink)?; - result.geometry = sink.geometry; - Ok(result) - } - pub fn parse_with_limits_and_sink<'src>( request: AdapterRequest<'src>, limits: LdrawLimits, @@ -93,20 +40,4 @@ impl InProcessRustAdapter { result.result.provenance_id = Cow::Owned(request.provenance_id.to_owned()); Ok(result) } - pub fn parse_fixture<'a>( - fixture_id: &'a str, - bytes: &'a [u8], - profile: ParserProfile, - ) -> Result { - Self::parse(AdapterRequest { - bytes, - profile, - provenance_id: fixture_id, - root_name: fixture_id, - root: RootId::UploadedManifest, - materializations: &[], - target_selections: &[], - cancellation: CancellationPolicy::default(), - }) - } } diff --git a/crates/polymodel-ldraw-core/src/lib.rs b/crates/polymodel-ldraw-core/src/lib.rs index 5caec9a..b68fbfb 100644 --- a/crates/polymodel-ldraw-core/src/lib.rs +++ b/crates/polymodel-ldraw-core/src/lib.rs @@ -33,6 +33,113 @@ mod tests { fn options<'a>() -> ParseOptions<'a> { ParseOptions::new() } + + #[derive(Default)] + struct RecordingSink { + geometry: std::collections::HashMap>, + } + + impl SemanticSink for RecordingSink { + fn on_event(&mut self, event: &SemanticEvent<'_>, _context: &TraversalContext) { + let SemanticEvent::GeometryEmitted { + primitive_id, + source, + span, + line_type, + colour, + vertices, + bfc, + texmap, + inverted, + } = event + else { + return; + }; + self.geometry + .entry(source.clone()) + .or_default() + .push(GeometryRecord { + primitive_id: *primitive_id, + source: source.clone(), + span: *span, + line_type: *line_type, + colour: *colour, + vertices: vertices.to_vec(), + bfc: *bfc, + texmap: texmap.cloned(), + inverted: *inverted, + }); + } + } + + #[derive(Debug)] + struct TestParseResult { + result: OwnedParseResult, + geometry: std::collections::HashMap>, + } + + impl std::ops::Deref for TestParseResult { + type Target = OwnedParseResult; + + fn deref(&self) -> &Self::Target { + &self.result + } + } + + trait LdrawParserTestExt { + fn parse_bytes<'src, 'a>( + &self, + bytes: &'src [u8], + options: ParseOptions<'a>, + materializations: &[Materialization], + ) -> Result; + } + + impl LdrawParserTestExt for LdrawParser { + fn parse_bytes<'src, 'a>( + &self, + bytes: &'src [u8], + options: ParseOptions<'a>, + materializations: &[Materialization], + ) -> Result { + let mut sink = RecordingSink::default(); + let result = self.parse_bytes_with_sink(bytes, options, materializations, &mut sink)?; + Ok(TestParseResult { + result, + geometry: sink.geometry, + }) + } + } + + fn adapter_parse<'src>(request: AdapterRequest<'src>) -> Result { + let mut sink = RecordingSink::default(); + let result = InProcessRustAdapter::parse_with_limits_and_sink( + request, + LdrawLimits::default(), + &mut sink, + )?; + Ok(TestParseResult { + result, + geometry: sink.geometry, + }) + } + + fn adapter_parse_fixture<'a>( + fixture_id: &'a str, + bytes: &'a [u8], + profile: ParserProfile, + ) -> Result { + adapter_parse(AdapterRequest { + bytes, + profile, + provenance_id: fixture_id, + root_name: fixture_id, + root: RootId::UploadedManifest, + materializations: &[], + target_selections: &[], + cancellation: CancellationPolicy::default(), + }) + } #[test] fn normalized_paths_reject_rooted_include_forms() { for path in [ @@ -125,7 +232,7 @@ mod tests { &[], ) .expect("compatibility profile should continue after invalid colour"); - assert_eq!(event_geometry(&result, &result.models[0]).len(), 1); + assert_eq!(event_geometry(&result.geometry, &result.models[0]).len(), 1); assert_eq!(result.models[0].triangles, 1); assert_eq!(result.models[0].quads, 0); } @@ -143,7 +250,7 @@ mod tests { true, Some("caller-materialization".into()), ); - let result = InProcessRustAdapter::parse(AdapterRequest { + let result = adapter_parse(AdapterRequest { bytes, profile: ParserProfile::Compatibility, provenance_id: "caller-provenance", @@ -219,7 +326,7 @@ mod tests { let result = LdrawParser .parse_bytes(b"5 16 0 0 0 1 0 0 0 1 0 2 0 0\n", options(), &[]) .unwrap(); - let model_geometry = event_geometry(&result, &result.models[0]); + let model_geometry = event_geometry(&result.geometry, &result.models[0]); let geometry = &model_geometry[0]; let (endpoints, controls) = geometry .conditional_endpoints() @@ -653,7 +760,7 @@ mod tests { true, None, ); - let result = InProcessRustAdapter::parse(AdapterRequest { + let result = adapter_parse(AdapterRequest { bytes, profile: ParserProfile::Compatibility, provenance_id: "occurrence-state", @@ -683,7 +790,7 @@ mod tests { polymodel_ldraw_testkit::Profile::Compatibility => ParserProfile::Compatibility, polymodel_ldraw_testkit::Profile::Lossless => ParserProfile::Lossless, }; - let result = InProcessRustAdapter::parse_fixture(fixture.id, fixture.bytes, profile); + let result = adapter_parse_fixture(fixture.id, fixture.bytes, profile); match fixture.expected.outcome { polymodel_ldraw_testkit::ExpectedOutcome::Rejected => { assert_eq!( @@ -817,9 +924,8 @@ mod tests { #[test] fn syntax_projection_comparison_rejects_empty_and_mutated_records() { let fixture = corpus_fixture("syntax-types"); - let parsed = - InProcessRustAdapter::parse_fixture(fixture.id, fixture.bytes, ParserProfile::Strict) - .expect("syntax fixture should parse"); + let parsed = adapter_parse_fixture(fixture.id, fixture.bytes, ParserProfile::Strict) + .expect("syntax fixture should parse"); let projected = parsed.project(); let empty = Vec::new(); let error = polymodel_ldraw_testkit::compare_syntax_records( @@ -903,12 +1009,11 @@ mod tests { } } - fn event_geometry(result: &OwnedParseResult, model: &ModelSummary) -> Vec { - result - .geometry - .get(&model.cache_key) - .cloned() - .unwrap_or_default() + fn event_geometry( + geometry: &std::collections::HashMap>, + model: &ModelSummary, + ) -> Vec { + geometry.get(&model.cache_key).cloned().unwrap_or_default() } fn assert_scene_counts( @@ -927,13 +1032,13 @@ mod tests { ); } - fn assert_sticker_geometry(result: &OwnedParseResult) { + fn assert_sticker_geometry(result: &TestParseResult) { let sticker = result .models .iter() .find(|model| model.path == "sticker.ldr") .expect("sticker sub-model"); - let sticker_geometry = event_geometry(result, sticker); + let sticker_geometry = event_geometry(&result.geometry, sticker); assert_geometry( &sticker_geometry, &[( @@ -950,7 +1055,7 @@ mod tests { for model in &result.models { if model.path != "sticker.ldr" { assert!( - event_geometry(result, model).is_empty(), + event_geometry(&result.geometry, model).is_empty(), "non-sticker model {} unexpectedly contains geometry", model.path ); @@ -962,8 +1067,7 @@ mod tests { fn corpus_bfc_transitions_from_certified_to_uncertified() { let fixture = corpus_fixture("bfc-invertnext"); let result = - InProcessRustAdapter::parse_fixture(fixture.id, fixture.bytes, ParserProfile::Strict) - .unwrap(); + adapter_parse_fixture(fixture.id, fixture.bytes, ParserProfile::Strict).unwrap(); assert_eq!(result.models.len(), 1); assert_eq!(result.models[0].bfc, BfcState::Uncertified); assert_eq!(result.semantic.bfc_state, BfcState::Uncertified); @@ -979,7 +1083,7 @@ mod tests { assert_scene_counts(&result, 2, 0, 0, 0); assert_geometry( - &event_geometry(&result, &result.models[0]), + &event_geometry(&result.geometry, &result.models[0]), &[ ( 3, @@ -1007,8 +1111,7 @@ mod tests { fn corpus_mpd_contains_two_virtual_files() { let fixture = corpus_fixture("mpd-data"); let result = - InProcessRustAdapter::parse_fixture(fixture.id, fixture.bytes, ParserProfile::Strict) - .unwrap(); + adapter_parse_fixture(fixture.id, fixture.bytes, ParserProfile::Strict).unwrap(); assert_eq!(result.files.len(), 3); assert_eq!( result @@ -1077,7 +1180,7 @@ mod tests { assert_eq!(start.reference.as_deref(), Some("sticker.png")); assert_scene_counts(&result, 2, 1, 0, 0); assert_geometry( - &event_geometry(&result, &result.models[0]), + &event_geometry(&result.geometry, &result.models[0]), &[ ( 3, @@ -1127,7 +1230,7 @@ mod tests { let result = LdrawParser .parse_bytes(bytes.as_bytes(), options(), &[]) .expect("valid bang-colon geometry should parse"); - let model_geometry = event_geometry(&result, &result.models[0]); + let model_geometry = event_geometry(&result.geometry, &result.models[0]); assert_eq!( model_geometry.len(), usize::from(expected_type != LineType::One) @@ -1159,7 +1262,7 @@ mod tests { &[], ) .expect("fallback bang-colon geometry should parse"); - let model_geometry = event_geometry(&result, &result.models[0]); + let model_geometry = event_geometry(&result.geometry, &result.models[0]); assert!(model_geometry[0].texmap.as_ref().unwrap().fallback); } @@ -1189,7 +1292,7 @@ mod tests { .iter() .any(|diagnostic| diagnostic.code == code) ); - assert!(event_geometry(&result, &result.models[0]).is_empty()); + assert!(event_geometry(&result.geometry, &result.models[0]).is_empty()); } } @@ -1260,7 +1363,7 @@ mod tests { result.semantic.texmap_events[0].reference.as_deref(), Some("next.png") ); - let model_geometry = event_geometry(&result, &result.models[0]); + let model_geometry = event_geometry(&result.geometry, &result.models[0]); assert_eq!( model_geometry[0] .texmap @@ -1291,7 +1394,7 @@ mod tests { .iter() .any(|diagnostic| diagnostic.code == DiagnosticCode::InvalidColour) ); - let model_geometry = event_geometry(&result, &result.models[0]); + let model_geometry = event_geometry(&result.geometry, &result.models[0]); assert_eq!(model_geometry.len(), 1); assert_eq!( model_geometry[0] @@ -1327,7 +1430,7 @@ mod tests { ); assert_scene_counts(&result, 1, 1, 1, 1); assert_geometry( - &event_geometry(&result, &result.models[0]), + &event_geometry(&result.geometry, &result.models[0]), &[ ( 2, @@ -1508,7 +1611,7 @@ mod tests { assert_eq!(result.semantic.colour_state, crate::ColourCode(301)); assert_scene_counts(&result, 2, 0, 0, 0); assert_geometry( - &event_geometry(&result, &result.models[0]), + &event_geometry(&result.geometry, &result.models[0]), &[ ( 3, @@ -1535,12 +1638,8 @@ mod tests { #[test] fn external_ldr_tools_step_ends_match_oracle() { let fixture = corpus_fixture("external-ldr-tools"); - let result = InProcessRustAdapter::parse_fixture( - fixture.id, - fixture.bytes, - ParserProfile::Compatibility, - ) - .expect("external-ldr-tools should parse"); + let result = adapter_parse_fixture(fixture.id, fixture.bytes, ParserProfile::Compatibility) + .expect("external-ldr-tools should parse"); let oracle = &fixture.expected.oracle_expected; assert_eq!(result.models.len(), oracle.models.len()); let root = oracle.root_model.as_ref().expect("oracle root model"); @@ -1599,12 +1698,9 @@ mod tests { ) in cases { let fixture = corpus_fixture(fixture_id); - let result = InProcessRustAdapter::parse_fixture( - fixture.id, - fixture.bytes, - ParserProfile::Compatibility, - ) - .unwrap_or_else(|e| panic!("{fixture_id} should parse: {e}")); + let result = + adapter_parse_fixture(fixture.id, fixture.bytes, ParserProfile::Compatibility) + .unwrap_or_else(|e| panic!("{fixture_id} should parse: {e}")); let target = result .semantic diff --git a/crates/polymodel-ldraw-core/src/model.rs b/crates/polymodel-ldraw-core/src/model.rs index a304931..cab6bc7 100644 --- a/crates/polymodel-ldraw-core/src/model.rs +++ b/crates/polymodel-ldraw-core/src/model.rs @@ -785,9 +785,6 @@ pub struct ParseResult<'src> { /// one object: projection borrows or clones data while this owner remains live. pub struct OwnedParseResult { pub result: ParseResult<'static>, - /// Compatibility geometry for callers that adapt after parsing. Worker parsing - /// uses a SemanticSink and does not read this field. - pub geometry: std::collections::HashMap>, } impl fmt::Debug for OwnedParseResult { @@ -811,7 +808,6 @@ impl OwnedParseResult { pub(crate) fn from_parse_result(result: ParseResult<'_>) -> Self { Self { result: result.into_owned(), - geometry: std::collections::HashMap::new(), } } diff --git a/crates/polymodel-ldraw-core/src/parser.rs b/crates/polymodel-ldraw-core/src/parser.rs index 1280f3b..64435b6 100644 --- a/crates/polymodel-ldraw-core/src/parser.rs +++ b/crates/polymodel-ldraw-core/src/parser.rs @@ -80,23 +80,6 @@ impl Default for LdrawParser { } } impl LdrawParser { - /// Parse immutable root bytes plus caller-admitted materializations. - /// - /// Discovery, fetching, precedence, and host policy are intentionally not - /// part of this core API. The adapter must provide every materialization it - /// wants the deterministic parser to consider. - pub fn parse_bytes<'src, 'a>( - &self, - bytes: &'src [u8], - options: ParseOptions<'a>, - materializations: &[Materialization], - ) -> Result { - let mut sink = crate::CompatibilitySink::default(); - let mut result = self.parse_bytes_with_sink(bytes, options, materializations, &mut sink)?; - result.geometry = sink.geometry; - Ok(result) - } - pub fn parse_bytes_with_sink<'src, 'a>( &self, bytes: &'src [u8], diff --git a/tools/ldraw-compat-harness/src/main.rs b/tools/ldraw-compat-harness/src/main.rs index e947e54..1b4b5b6 100644 --- a/tools/ldraw-compat-harness/src/main.rs +++ b/tools/ldraw-compat-harness/src/main.rs @@ -1,6 +1,7 @@ //! Native PM-77 corpus validator and D-phase LDParse oracle harness. use polymodel_ldraw_core::{ - AdapterRequest, CancellationPolicy, InProcessRustAdapter, ParserProfile as CoreProfile, RootId, + AdapterRequest, CancellationPolicy, InProcessRustAdapter, ParserProfile as CoreProfile, + RootId, SemanticEvent, SemanticSink, CacheKey, GeometryRecord, OwnedParseResult, ParseError, }; use polymodel_ldraw_testkit::{ CorpusGateSet, GATE_NAMES, LDRAW_LIMITS, ORACLE_PATCH_IDENTITY, ORACLE_PROJECTION_RULE_VERSION, @@ -243,7 +244,7 @@ fn corpus() -> Result { if fixture.outcome == polymodel_ldraw_testkit::ExpectedOutcome::Cancelled { let cancellation = CancellationPolicy::new(); cancellation.cancel_after_checkpoints(1); - InProcessRustAdapter::parse(polymodel_ldraw_core::AdapterRequest { + harness_parse(polymodel_ldraw_core::AdapterRequest { bytes: fixture.bytes, profile: core_profile(fixture.profile), provenance_id: fixture.id, @@ -276,7 +277,8 @@ fn corpus() -> Result { | (polymodel_ldraw_testkit::ExpectedOutcome::Cancelled, Ok(_)) => { return Err(format!("{} unexpectedly accepted", fixture.id)); } - (_, Ok(parsed)) => { + (_, Ok(harness)) => { + let parsed = harness.result; let actual = parsed.project(); let results = gates.evaluate_with_files(fixture, &actual, &parsed.files); if results.is_empty() { @@ -776,8 +778,8 @@ fn fixture_root_name(fixture: &polymodel_ldraw_testkit::FixtureCase) -> &str { fn parse_fixture_with_root( fixture: &polymodel_ldraw_testkit::FixtureCase, -) -> Result { - InProcessRustAdapter::parse(AdapterRequest { +) -> Result { + harness_parse(AdapterRequest { bytes: fixture.bytes, profile: core_profile(fixture.profile), provenance_id: fixture.id, @@ -823,6 +825,92 @@ fn candidate_observation( })) } +struct HarnessSink { + geometry: std::collections::HashMap>, +} + +impl Default for HarnessSink { + fn default() -> Self { + Self { + geometry: std::collections::HashMap::new(), + } + } +} + +impl SemanticSink for HarnessSink { + fn on_event(&mut self, event: &SemanticEvent<'_>, _context: &polymodel_ldraw_core::TraversalContext) { + if let SemanticEvent::GeometryEmitted { + source, + line_type, + colour, + vertices, + bfc, + texmap, + inverted, + primitive_id, + span, + } = event + { + self.geometry.entry(source.clone()).or_default().push(GeometryRecord { + primitive_id: *primitive_id, + source: source.clone(), + span: *span, + line_type: *line_type, + colour: *colour, + vertices: vertices.to_vec(), + bfc: *bfc, + texmap: texmap.cloned(), + inverted: *inverted, + }); + } + } +} + +struct HarnessParseResult { + result: OwnedParseResult, + geometry: std::collections::HashMap>, +} + +fn harness_parse(request: AdapterRequest<'_>) -> Result { + let mut sink = HarnessSink::default(); + let result = InProcessRustAdapter::parse_with_limits_and_sink( + request, + polymodel_ldraw_core::LdrawLimits::default(), + &mut sink, + )?; + Ok(HarnessParseResult { + result, + geometry: sink.geometry, + }) +} + +fn harness_parse_fixture( + provenance: &str, + bytes: &[u8], + profile: CoreProfile, +) -> Result { + let mut sink = HarnessSink::default(); + let request = AdapterRequest { + bytes, + profile, + provenance_id: provenance, + root_name: provenance, + root: RootId::UploadedManifest, + materializations: &[], + target_selections: &[], + cancellation: CancellationPolicy::default(), + }; + let result = InProcessRustAdapter::parse_with_limits_and_sink( + request, + polymodel_ldraw_core::LdrawLimits::default(), + &mut sink, + )?; + Ok(HarnessParseResult { + result, + geometry: sink.geometry, + }) +} + fn build_semantic_from_geometry( geometry: &[polymodel_ldraw_core::GeometryRecord], ) -> polymodel_ldraw_core::LdMeshSemantic { @@ -861,10 +949,11 @@ fn compare_candidate_to_oracle( oracle: &OracleProjection, attestations: &mut Vec, ) -> Result<(), String> { - let parsed = parse_fixture_with_root( + let harness = parse_fixture_with_root( fixture, ) .map_err(|error| format!("{} candidate rejected while oracle status is {}: {error}", fixture.id, oracle.status))?; + let parsed = &harness.result; let candidate = parsed.project(); if oracle.status != "ok" { if oracle.status != "error" @@ -1014,7 +1103,7 @@ fn compare_candidate_to_oracle( )); } if oracle_model.mesh.vertex_count > 0 { - let semantic = parsed + let semantic = harness .geometry .get(&candidate_model.cache_key) .map(|geometry| build_semantic_from_geometry(geometry)) @@ -1283,7 +1372,7 @@ fn fuzz() -> Result { fs::write(&path, &bytes).map_err(|error| error.to_string())?; let operator = mutation_operator(FUZZ_SEED, index); let first = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { - InProcessRustAdapter::parse_fixture( + harness_parse_fixture( fixture.id, &bytes, core_profile(fixture.profile), @@ -1296,7 +1385,7 @@ fn fuzz() -> Result { ) })?; let first_observation = match first { - Ok(parsed) => candidate_observation(&parsed).and_then(|observation| { + Ok(harness) => candidate_observation(&harness.result).and_then(|observation| { serde_json::to_vec(&observation) }).map_err(|error| { format!( @@ -1307,7 +1396,7 @@ fn fuzz() -> Result { Err(error) => Err(format!("error:{error:?}")), }; let repeat = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { - InProcessRustAdapter::parse_fixture( + harness_parse_fixture( fixture.id, &bytes, core_profile(fixture.profile), @@ -1320,7 +1409,7 @@ fn fuzz() -> Result { ) })?; let second_observation = match repeat { - Ok(parsed) => candidate_observation(&parsed).and_then(|observation| { + Ok(harness) => candidate_observation(&harness.result).and_then(|observation| { serde_json::to_vec(&observation) }).map_err(|error| { format!( -- knotmirror 0.0.1