Skip to content

Commit 2e3dd48

Browse files
committed
fix(lance-graph-ontology): resolve Codex PR #407 P1 + P2 review findings
P1 — SKR03 Bau no longer overwrites canonical SKR03 slot: Before: hydrate_skr03_bau registered into OGIT::SKR03_V1 (same slot as canonical), so a caller that hydrated both lost the canonical 4-digit account set on the second call. The test only invoked Bau alone so the silent overwrite went undetected. After: SKR 03 Bau hydrates into its OWN G slot OGIT::SKR03BAU_V1=42, with inherits_from: Some(OGIT::SKR03_V1.0) to make the structural dependency on canonical SKR 03 explicit. Mixed consumers can now hold canonical SKR 03 (4-digit, slot 40) AND Bau (6-digit, slot 42) in the same OntologyRegistry without interference. - crates/lance-graph-contract/build.rs: allocates SKR03BAU=42 - crates/lance-graph-ontology/src/hydrators/skr_datev.rs: hydrate_skr03_bau writes to OGIT::SKR03BAU_V1.0 with the new inherits_from - modules/skr03-bau/manifest.yaml: declares 5 anchor trade-specific accounts (Sand- und Kiesausbeute, Bauliche Anlagen für stationäre Fertigung, Bauten auf dem Bauhof) with stable u16 entity IDs - tests: skr03_bau_extensions_hydrate_into_skr03_slot renamed to skr03_bau_extensions_hydrate_into_dedicated_slot (expects slot 42) - NEW REGRESSION TEST: skr03_canonical_and_bau_coexist_in_one_registry hydrates both schemes in sequence, asserts: - canonical bundle still has 1400+ entities (not dropped) - canonical SKR 03/1000 (Kasse) resolves in canonical bundle - Bau /007510 (Sand- und Kiesausbeute) resolves in Bau bundle - Bau IRIs do NOT resolve in canonical bundle (and vice versa) P2 — Schematron Event::Empty no longer leaks state into later text: Before: <assert .../>, <report .../>, <pattern .../> (self-closing) emitted Event::Empty handled with the same branch as Event::Start and set in_assert_or_report = true. Self-closing elements never emit a matching Event::End, so the flag stayed stuck true and the parser scanned later unrelated text body as rule message text, producing spurious /rule/... IRIs. After: Event::Start and Event::Empty are split into separate match arms. Empty interns the @id only (no body to collect) and does NOT touch in_assert_or_report. Start sets the flag; End extracts rule IDs from current_text_buf and resets state. Self-closing elements no longer affect state. - crates/lance-graph-ontology/src/hydrators/schematron.rs - NEW REGRESSION TEST: self_closing_assert_does_not_capture_later_text constructs a fixture with a self-closing <assert/> followed by a non-assert <bar> element containing "[BR-99]" text, then a normal Start/End assert containing "[BR-42]": - A-EMPTY-001 and A-NORMAL-001 assert IRIs both resolve - BR-42 (real rule from message body) resolves - BR-99 (from stray text outside any assert) does NOT resolve Side fix: replaced `if !map.contains_key(&iri) { map.insert(iri, id) }` in SkrHydrator with the Entry::Vacant pattern to clear a clippy `map_entry` warning. Clippy back to 5 warnings (all pre-existing oxrdf deprecation warnings, no new ones). All 116 lance-graph-ontology tests pass (was 114; +2 regression tests); downstream consumers build clean.
1 parent 299e6c4 commit 2e3dd48

6 files changed

Lines changed: 204 additions & 21 deletions

File tree

crates/lance-graph-contract/build.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,12 @@ const CANONICAL_SLOTS: &[(&str, u32)] = &[
5555
// in SKR 04 but "Kasse" in SKR 03).
5656
("SKR03", 40),
5757
("SKR04", 41),
58+
// SKR 03 Bau und Handwerk (Branchenpaket 19606) — trade-specific
59+
// 6-digit extensions on top of canonical SKR 03 (Sand- und
60+
// Kiesausbeute, Bauliche Anlagen, etc.). Hydrates into its OWN G
61+
// slot rather than the canonical SKR03_V1 slot so mixed consumers
62+
// can hold both account sets in one OntologyRegistry.
63+
("SKR03BAU", 42),
5864
];
5965

6066
fn canonical_slot(token: &str) -> Option<u32> {

crates/lance-graph-ontology/src/hydrators/schematron.rs

Lines changed: 95 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ fn walk_sch(
128128
});
129129
}
130130
Ok(Event::Eof) => break,
131-
Ok(Event::Start(e)) | Ok(Event::Empty(e)) => {
131+
Ok(Event::Start(e)) => {
132132
let qname = e.name();
133133
let local: Vec<u8> = local_name(qname.as_ref()).to_vec();
134134
let id = attr_value(&e, b"id");
@@ -158,6 +158,38 @@ fn walk_sch(
158158
_ => {}
159159
}
160160
}
161+
// Self-closing `<assert .../>` / `<report .../>` / `<pattern .../>`
162+
// emit Event::Empty WITHOUT a matching Event::End. We MUST NOT set
163+
// `in_assert_or_report = true` here — there is no text body to
164+
// collect, and leaving the flag stuck true causes downstream
165+
// unrelated text to be scanned as rule text and produce spurious
166+
// `/rule/...` IRIs (Codex P2 finding, PR #407 review).
167+
Ok(Event::Empty(e)) => {
168+
let qname = e.name();
169+
let local: Vec<u8> = local_name(qname.as_ref()).to_vec();
170+
let id = attr_value(&e, b"id");
171+
match local.as_slice() {
172+
b"assert" => {
173+
if let Some(id) = id.as_deref() {
174+
let iri = format!("{base_iri}/assert/{id}");
175+
intern(iri, iri_to_id, next_id);
176+
}
177+
}
178+
b"report" => {
179+
if let Some(id) = id.as_deref() {
180+
let iri = format!("{base_iri}/report/{id}");
181+
intern(iri, iri_to_id, next_id);
182+
}
183+
}
184+
b"pattern" => {
185+
if let Some(id) = id.as_deref() {
186+
let iri = format!("{base_iri}/pattern/{id}");
187+
intern(iri, iri_to_id, next_id);
188+
}
189+
}
190+
_ => {}
191+
}
192+
}
161193
Ok(Event::Text(t)) if in_assert_or_report => {
162194
if let Ok(s) = std::str::from_utf8(t.as_ref()) {
163195
current_text_buf.push_str(s);
@@ -348,4 +380,66 @@ mod tests {
348380
let ids = extract_business_rule_ids(s);
349381
assert_eq!(ids, vec!["BR-45".to_string()]);
350382
}
383+
384+
const SELF_CLOSING_SCH: &str = r#"<?xml version="1.0" encoding="UTF-8"?>
385+
<schema xmlns="http://purl.oclc.org/dsdl/schematron" queryBinding="xslt2">
386+
<pattern id="P-empty">
387+
<rule context="//foo">
388+
<!-- Self-closing assert: no message body, no End event. -->
389+
<assert id="A-EMPTY-001" test="bar"/>
390+
<!-- After the self-closing assert, this stray text mentioning
391+
[BR-99] in a non-assert/non-report context MUST NOT be
392+
picked up as a rule IRI. -->
393+
<bar>Stray [BR-99] text outside any assert</bar>
394+
<!-- Normal Start/End assert AFTER the empty: must work as usual. -->
395+
<assert id="A-NORMAL-001" test="baz">[BR-42]-Real rule.</assert>
396+
</rule>
397+
</pattern>
398+
</schema>
399+
"#;
400+
401+
#[test]
402+
fn self_closing_assert_does_not_capture_later_text() {
403+
// Regression test for the Codex P2 finding (PR #407 review):
404+
// `<assert .../>` emits Event::Empty with no matching End. Before
405+
// the fix, `in_assert_or_report` was set true and never reset,
406+
// so subsequent text bodies were scanned as rule message text
407+
// and produced spurious /rule/... IRIs.
408+
let dir = tempdir().unwrap();
409+
let path = dir.path().join("self-closing.sch");
410+
let mut f = std::fs::File::create(&path).unwrap();
411+
f.write_all(SELF_CLOSING_SCH.as_bytes()).unwrap();
412+
drop(f);
413+
414+
let reg = OntologyRegistry::new_in_memory();
415+
let h = SchematronHydrator {
416+
g: 6666,
417+
version: 1,
418+
domain_name: "self-closing".to_string(),
419+
inherits_from: None,
420+
starting_entity_id: 100,
421+
base_iri: "urn:schematron:test".to_string(),
422+
};
423+
h.hydrate_many(&[&path], &reg).expect("hydrate");
424+
425+
let bundle = reg.bundle_for(6666).expect("bundle");
426+
427+
// Assert IDs from both the empty and the normal assert resolve.
428+
assert!(bundle.resolve_iri("urn:schematron:test/assert/A-EMPTY-001").is_some());
429+
assert!(bundle.resolve_iri("urn:schematron:test/assert/A-NORMAL-001").is_some());
430+
assert!(bundle.resolve_iri("urn:schematron:test/pattern/P-empty").is_some());
431+
432+
// BR-42 from the NORMAL assert's body must resolve (positive control).
433+
assert!(
434+
bundle.resolve_iri("urn:schematron:test/rule/BR-42").is_some(),
435+
"BR-42 from the normal assert's message body must resolve"
436+
);
437+
438+
// BR-99 from text OUTSIDE any assert/report must NOT resolve.
439+
assert!(
440+
bundle.resolve_iri("urn:schematron:test/rule/BR-99").is_none(),
441+
"BR-99 was in stray text outside an assert; must NOT have been \
442+
interned as a rule IRI (this was the P2 corruption case)"
443+
);
444+
}
351445
}

crates/lance-graph-ontology/src/hydrators/skr.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,10 @@ impl SkrHydrator {
9191
continue;
9292
}
9393
let iri = format!("{}/{}", self.iri_prefix, account_number);
94-
if !iri_to_id.contains_key(&iri) {
94+
if let std::collections::hash_map::Entry::Vacant(slot) = iri_to_id.entry(iri) {
9595
let id = next_id;
9696
next_id += 1;
97-
iri_to_id.insert(iri, id);
97+
slot.insert(id);
9898
}
9999
}
100100

crates/lance-graph-ontology/src/hydrators/skr_datev.rs

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,21 @@
1-
//! Wiring for the two DATEV SKR schemes.
1+
//! Wiring for the DATEV SKR schemes.
22
//!
33
//! - `hydrate_skr03` → `OGIT::SKR03_V1`, base IRI `urn:datev:skr03:account`.
44
//! Reads `data/ontologies/skr-datev/skr03.csv` (canonical 4-digit accounts).
55
//! - `hydrate_skr04` → `OGIT::SKR04_V1`, base IRI `urn:datev:skr04:account`.
66
//! Reads `data/ontologies/skr-datev/skr04.csv`.
7-
//! - `hydrate_skr03_bau` → also `OGIT::SKR03_V1`, but with the 6-digit Bau
8-
//! variant. Test-only entry point; the default `hydrate_skr03` uses the
9-
//! canonical 4-digit form because cross-scheme alignment (FIBO, ZUGFeRD)
10-
//! anchors to 4-digit numbers.
7+
//! - `hydrate_skr03_bau` → `OGIT::SKR03BAU_V1` (slot 42), base IRI
8+
//! `urn:datev:skr03-bau:account`. Reads
9+
//! `data/ontologies/skr-datev/skr03-bau.csv` with the 6-digit Bau-und-Handwerk
10+
//! trade-specific subdivisions. Lives in its OWN G slot (not SKR03_V1)
11+
//! so callers can hold BOTH canonical SKR 03 AND Bau extensions in one
12+
//! `OntologyRegistry` without one overwriting the other. The Bau slot
13+
//! declares `inherits_from: Some(OGIT::SKR03_V1.0)` to make the
14+
//! structural dependency explicit.
1115
//!
12-
//! Both schemes declare `inherits_from: Some(OGIT::DOLCE_V1.0)` — accounts
13-
//! are abstract economic objects, anchored to DUL Object via the cognitive
14-
//! shader's downstream alignment axioms (not baked in here).
16+
//! SKR 03 and SKR 04 each declare `inherits_from: Some(OGIT::DOLCE_V1.0)` —
17+
//! accounts are abstract economic objects, anchored to DUL Object via the
18+
//! cognitive shader's downstream alignment axioms (not baked in here).
1519
1620
use std::path::{Path, PathBuf};
1721

@@ -64,15 +68,15 @@ pub fn hydrate_skr03_bau_from(
6468
registry: &OntologyRegistry,
6569
) -> Result<u32, HydrateErr> {
6670
let h = SkrHydrator {
67-
g: OGIT::SKR03_V1.0,
68-
version: OGIT::SKR03_V1.1,
71+
g: OGIT::SKR03BAU_V1.0,
72+
version: OGIT::SKR03BAU_V1.1,
6973
domain_name: "skr03-bau".to_string(),
70-
inherits_from: Some(OGIT::DOLCE_V1.0),
74+
inherits_from: Some(OGIT::SKR03_V1.0),
7175
starting_entity_id: 100,
7276
iri_prefix: SKR03_BAU_IRI_PREFIX.to_string(),
7377
};
7478
h.hydrate(csv_path, registry)?;
75-
Ok(OGIT::SKR03_V1.0)
79+
Ok(OGIT::SKR03BAU_V1.0)
7680
}
7781

7882
/// Hydrate SKR 04 (4-digit accounts, balance-sheet-oriented) as `OGIT::SKR04_V1`.

crates/lance-graph-ontology/tests/skr_hydrator_smoke.rs

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -146,16 +146,24 @@ fn skr03_and_skr04_are_separate_slots() {
146146
}
147147

148148
#[test]
149-
fn skr03_bau_extensions_hydrate_into_skr03_slot() {
150-
// The Bau variant uses a SEPARATE IRI prefix (urn:datev:skr03-bau:…) so
151-
// its 6-digit accounts coexist with the canonical 4-digit set. Bau
152-
// extensions appear at sub-account numbers like 007510 (Sand- und
153-
// Kiesausbeute), 010010 (Bauliche Anlagen für stationäre Fertigung).
149+
fn skr03_bau_extensions_hydrate_into_dedicated_slot() {
150+
// Bau hydrates into ITS OWN G slot (OGIT::SKR03BAU_V1 = 42), NOT the
151+
// canonical SKR03_V1 slot, so a caller that hydrates BOTH canonical
152+
// and Bau in the same OntologyRegistry holds both bundles
153+
// independently without one overwriting the other. The Bau bundle
154+
// declares inherits_from: Some(OGIT::SKR03_V1.0) to make the
155+
// structural dependency on canonical SKR 03 explicit.
154156
let registry = OntologyRegistry::new_in_memory();
155157
let g = hydrate_skr03_bau(&registry).expect("SKR 03 Bau hydrates");
156-
assert_eq!(g, OGIT::SKR03_V1.0);
158+
assert_eq!(g, OGIT::SKR03BAU_V1.0);
157159

158160
let bundle = registry.bundle_for(g).expect("bundle");
161+
assert_eq!(bundle.domain_name, "skr03-bau");
162+
assert_eq!(
163+
bundle.inherits_from,
164+
Some(OGIT::SKR03_V1.0),
165+
"Bau must inherit from canonical SKR 03"
166+
);
159167
// 1686 in the Bau CSV. Allow a bit of slack.
160168
assert!(
161169
bundle.entity_count() >= 1600,
@@ -171,3 +179,44 @@ fn skr03_bau_extensions_hydrate_into_skr03_slot() {
171179
);
172180
}
173181
}
182+
183+
#[test]
184+
fn skr03_canonical_and_bau_coexist_in_one_registry() {
185+
// Regression test for the Codex P1 finding (PR #407 review):
186+
// hydrate_skr03 + hydrate_skr03_bau in sequence must populate TWO
187+
// distinct G slots, not overwrite each other. Before the fix the
188+
// second call dropped the canonical 4-digit accounts because both
189+
// hydrators registered into OGIT::SKR03_V1.
190+
let registry = OntologyRegistry::new_in_memory();
191+
hydrate_skr03(&registry).expect("canonical SKR 03");
192+
hydrate_skr03_bau(&registry).expect("SKR 03 Bau");
193+
194+
let canonical = registry.bundle_for(OGIT::SKR03_V1.0).expect("canonical bundle");
195+
let bau = registry.bundle_for(OGIT::SKR03BAU_V1.0).expect("Bau bundle");
196+
197+
// Canonical bundle: 4-digit IRI base, includes load-bearing accounts.
198+
assert!(
199+
canonical.resolve_iri(&format!("{SKR03_IRI_PREFIX}/1000")).is_some(),
200+
"canonical SKR 03 / 1000 Kasse must still resolve after Bau hydration"
201+
);
202+
assert!(
203+
canonical.entity_count() >= 1400,
204+
"canonical SKR 03 must retain its full account set after Bau hydration"
205+
);
206+
207+
// Bau bundle: 6-digit IRI base, includes trade-specific extensions.
208+
assert!(
209+
bau.resolve_iri(&format!("{SKR03_BAU_IRI_PREFIX}/007510")).is_some(),
210+
"Bau extension 007510 must resolve in Bau bundle"
211+
);
212+
213+
// Cross-resolve: Bau IRI must NOT resolve in canonical bundle and vice versa.
214+
assert!(
215+
canonical.resolve_iri(&format!("{SKR03_BAU_IRI_PREFIX}/007510")).is_none(),
216+
"Bau IRI must not resolve in canonical SKR 03 bundle"
217+
);
218+
assert!(
219+
bau.resolve_iri(&format!("{SKR03_IRI_PREFIX}/1000")).is_none(),
220+
"canonical SKR 03 IRI must not resolve in Bau bundle"
221+
);
222+
}

modules/skr03-bau/manifest.yaml

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
ogit_g: SKR03BAU
2+
version: 1
3+
domain_name: skr03-bau
4+
inert_when_consumer_absent: true
5+
6+
# DATEV SKR 03 Bau und Handwerk (Branchenpaket Art.-Nr. 19606, 2026
7+
# edition). 6-digit account format: NNNN + 2-digit trade-specific
8+
# subdivision. The full Bau CSV has 1880 accounts; 210 of these are
9+
# trade-specific extensions (NN > 00) on top of the canonical 1623
10+
# 4-digit accounts that ship in OGIT::SKR03_V1.
11+
#
12+
# Hydrates into its OWN G slot (NOT SKR03_V1) so mixed consumers can
13+
# hold both canonical SKR 03 AND Bau extensions in one OntologyRegistry.
14+
#
15+
# Anchor accounts here are trade-specific load-bearing entries.
16+
entity_types:
17+
# Substanzverzehr (raw-material extraction — Sand/Kies pits, quarries)
18+
Account_007510: u16=4200 # Sand- und Kiesausbeute
19+
Account_007511: u16=4201 # Grundstücke mit Substanzverzehr (Bau variant)
20+
# Bauliche Anlagen (construction-specific buildings)
21+
Account_010010: u16=4202 # Bauliche Anlagen für stationäre Fertigung
22+
Account_010011: u16=4203 # Fabrikbauten (Bau variant)
23+
# Bauten auf dem Bauhof (yard buildings)
24+
Account_011510: u16=4204 # Bauten auf dem Bauhof
25+
26+
rbac_policy: ~
27+
stack_profile: ~
28+
action_capabilities: {}
29+
actor: ~
30+
inherits_from: skr03

0 commit comments

Comments
 (0)