test(ontology): #204 OwlValidator live-path — silent false-pass is dead (dispatch 4rkh1s secondaire) - #482
Conversation
…SHelper silent-empty fallback Fixes the bug surfaced by PR #480 (OwlAdapter readers returning empty). Root cause was deeper than the .URI type-mismatch: RDFResource.Equals uses runtime-type comparison (GetType()), so even RDFResource.Equals(fresh RDFResource) is false when the stored IRI is a subtype — only GetIRI().Equals works. Switched all 14 reader comparison sites to .ToString() equality (the URI-as-string basis the write-path tests already proved correct, agnostic to RDFResource subtype). Also completes the "incomplete SKOSHelper bypass" flagged on the dashboard: CheckIsNarrowerConcept / GetExactMatch/Close/RelatedMatch had try/catch fallbacks that only triggered on exception, but SKOSHelper returns false/empty SILENTLY — so the (now-correct) fallback scanner was never reached. Changed fallback to trigger on empty/false result too, not just exception. Tests: section-(A) [BUG] characterization suite from #480 flipped to proper round-trip assertions (now the regression suite for this fix). Full suite 218 pass / 0 fail / 5 skip (baseline preserved). Production impact: Tests/OwlOntologyValidationTests.cs (post-gen OWL validator, not xUnit) was silently false-passing "no concepts → skip → PASS" on annotation and AIF checks because GetResourcesByType(Concept) returned empty. Now resolves correctly → validation becomes reliable (unblocks trustworthy #133 OWL publication). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ad (dispatch 4rkh1s secondaire) NEW additive file: OwlValidatorLivePathTests.cs (+5 tests). Dispatch `4rkh1s` secondaire — prove the production OWL validation path is LIVE after the #481 reader fix. The production validator `Tests/OwlOntologyValidationTests.cs` (runtime validator invoked by `OwlValidatorConfig.Apply`, NOT an xUnit suite) had a silent false-pass: ValidateMultilingualAnnotations and ValidateAIFMappings early-return `true` ("No concepts to validate — skipping") when GetResourcesByType(Concept) was empty. Before #481 that reader ALWAYS returned empty, so the validators reported PASS regardless of whether annotations/AIF mappings existed. These tests drive the REAL production validator via reflection (its `_ontology` field is private; the validation methods are public). They prove: - (1) annotated concepts → validator inspects them and reports genuine PASS (not the skip-path); - (2) UNannotated concepts → validator now FAILS (the annotation check actually ran and found the missing labels/definitions) — the dead silent false-pass is gone; - (3) the GetResourcesByType(Concept) predicate the validators branch on resolves non-empty. Tests: 223 pass / 0 fail / 5 skip (218 + 5). Deterministic, key-free, release-independent. No existing file modified. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
clusterManager-Myia
left a comment
There was a problem hiding this comment.
[NanoClaw]
Solid companion PR to #481. The fix is mechanical and correct; the test coverage is thorough. A few observations:
Production code (OwlAdapter.cs) - fix is correct
The core change across all 14 sites is identical in pattern: replace .Equals(string) with .ToString() == .ToString(). This works because RDFResource.Equals(object) dispatches by type and returns false when compared against a string or Uri - switching to string-string equality bypasses the type-mismatch.
Specific improvements beyond the raw .ToString() swap:
- CheckIsNarrowerConcept: The original
try { return ... } catch { return fallback }pattern meant the fallback never executed ifSKOSHelperreturnedfalse(no exception). The newtry { if (...) return true; } catch { } return fallbackcorrectly falls through on silentfalse, which is a real behavioral improvement - the annotation scanner now actually gets a chance to run. - GetExactMatchConcepts / GetCloseMatchConcepts / GetRelatedMatchConcepts: Same fix -
SKOSHelperreturningnullor empty no longer silently short-circuits; the annotation fallback now runs. This is the right pattern.
No issues with the production changes.
Regression tests (OwlAdapterRegressionTests.cs) - well-flipped
All 8 section-(A) tests that previously asserted the broken behavior (BeEmpty, BeFalse) are now proper round-trip assertions. The assertions are specific (HaveCount(3), BeEquivalentTo(...), ContainSingle(...)) rather than just NotBeEmpty, which is good.
The CheckIsNarrowerConcept test now also asserts the negative case (Unrelated returns false), and CheckHasClass asserts UndeclaredClass returns false. Good defensive coverage.
Live-path tests (OwlValidatorLivePathTests.cs) - well-designed with one structural concern
The test design is sound: prove the production validator actually runs (not just skips) by driving it through reflection. The 5 tests cover the two axes that matter:
- Annotated concepts -> PASS (genuine, not skip-based)
- Unannotated concepts -> FAIL (the silent false-pass is dead)
- AIF mappings -> same pattern for both PASS and FAIL
- Contract guard pinning
GetResourcesByType(Concept)returns non-empty
One concern - reflection fragility: BuildValidator accesses a private _ontology field by name. If the field is renamed, these tests silently break (compile error, not a test failure - but still invisible in CI if the test project isn't rebuilt). This is acceptable for now since OwlOntologyValidationTests has no public injection seam, but worth noting that a longer-term fix should add a constructor overload or factory method that accepts an OwlAdapter directly. Not blocking.
One minor observation: InvokeValidate casts the return to Task<bool> without checking for null. Low risk but could be hardened.
Security
No secrets, credentials, connection strings, or API keys in the diff.
Verdict
Clean fix, thorough regression suite, well-documented. The reflection-based approach for the live-path tests is the right pragmatic choice. Ship it.
…α resubmit) (#484) Re-submit the α deliverable from #444 (closed stale) as a clean doc-only PR. The original #444 bundled α (this assessment) + β (Memo Back loc fix = #446) + γ (gpt-5.5 PT task = #447). β and γ are already on master; γ additionally carried a parasite gpt-5.5→5.4-mini downgrade on 12 sites (avoided). The prior split branch (docs/444-alpha-dnn-upgrade-assessment) was based on pre-OWL-merge commit 36c138b and would have reverted #481/#482/#483 as parasites — this PR isolates the single new file on master c873bcd. Related: #131 (DNN security/upgrade), #132 (DNN deployment), #134 (release v0.9.0). Supersedes: #444 (closed stale; β=#446, γ=#447 already merged). Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…lent-false-pass bug (#486) Loads the committed generated ontology (docs/ontology/argumentum.owl, the real OwlDocumentConfig.CreateOwlDocument output) and runs the production validation path against it. This completes the #133 e2e proof that #482 (synthetic in-memory adapters) started — and surfaces a SECOND silent-false-pass bug the #480→#481→#482 lane missed because it only exercised the in-memory path. ROOT CAUSE (measured on the reloaded file): OWLSharp's OWL2XML serializer DROPS the rdf:type and skos:inScheme annotation assertions during serialization — neither survives the round-trip (rdf:type == 0, inScheme == 0 after reload). The OwlAdapter readers find concepts/schemes by scanning AnnotationAxioms for rdf:type, so on any LOADED file they return empty. OwlOntologyValidationTests.ValidateMultilingualAnnotations and .ValidateAIFMappings then hit their `if (concepts.Count == 0) return true;` guard and report PASS without inspecting anything. The real content IS present (2816 prefLabels, 10 AIF matches, 1510 class declarations) — the validator simply cannot see it. So #133's "confidence restored" premise does NOT hold for the production load-and-validate path: the silent false-pass is still alive there, for a different root cause than the #480 RDFResource type-mismatch. 4 characterization tests (all green, pinning current broken behavior): 1. rdf:type==0 and inScheme==0 after round-trip (prefLabel survives as contrast). 2. GetResourcesByType(Concept/ConceptScheme) returns empty on the loaded file. 3. The reloaded ontology DOES contain real content (prefLabel>1000, matches>0, classDecls>1000) — reader defect, not data defect. 4. Production validator returns TRUE for annotation + AIF checks on the loaded ontology — the silent false-pass, decisively pinned. When the fix lands (readers locate concepts via surviving annotations — prefLabel/definition/example subjects, or filtered DeclarationAxioms — NOT rdf:type/inScheme), these assertions flip to the honest "detection works" form. The fix is a prod behavior change with release-gate implications → coordinator scope; this PR surfaces it rather than shipping a unilateral prod change. Deterministic, key-free, release-independent. NEW additive file — nothing modified. Co-authored-by: Your <your.email@example.com> Co-authored-by: Claude <noreply@anthropic.com>
…nt-false-pass dead (#489) The #481 reader fix only covered the IN-MEMORY path. On a LOADED ontology (rdf:type + skos:inScheme dropped from the reloaded AnnotationAxioms by OWLSharp's OWL2XML round-trip), GetResourcesByType(Concept)/GetConcepts returned empty -> ValidateMultilingualAnnotations/ValidateAIFMappings hit their `concepts.Count == 0 -> return true` guard -> PASS without inspecting (the 2nd silent-false-pass, characterized by #486). Verified reloaded breakdown (probed on the real generated ontology): prefLabel=2816, definition=2816, example=2816, narrower/broader=1407, broadMatch=57, closeMatch=10, narrowMatch=3, hasTopConcept=1; rdf:type=0, inScheme=0 among AnnotationAxioms. Fix is READ-PATH ONLY (serializer untouched, per dispatch scope). When the rdf:type scan is empty, locate entities via the surviving SKOS annotations: - skos:Concept -> distinct subjects of skos:prefLabel (~1305 resolved) - skos:ConceptScheme -> subject of skos:hasTopConcept In-memory path preserved (rdf:type present -> early-return, no fallback), so the #482 in-memory live-path proofs still hold. Concepts deduped by URI string (not RDFResource.Equals) to avoid the equality bug class of #480. OwlE2EGenerationValidationTests (#486) flipped from bug-characterization to a genuine-pass regression suite: (1) rdf:type/inScheme drop still real (now benign), prefLabel survives (2) readers now resolve concepts (>1000) + scheme (NotBeEmpty) (3) real content now resolvable (>1000 distinct concept subjects) (4) prod validator genuinely inspects + passes (skip guard unreachable) Tests: Ontology namespace 25/25 green; full suite 259 passed / 0 failed / 5 skipped (no regression). Dispatch msg-...irkf5i primaire, base 4ac52e2. Read-path only: no write/serialize, no CSV/template/config touched. Co-authored-by: Your <your.email@example.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
OwlValidator live-path: silent false-pass is dead (dispatch
4rkh1ssecondaire)NEW additive file —
OwlValidatorLivePathTests.cs(+5 tests). Companion to PR #481 (the OwlAdapter reader fix). Dispatch4rkh1ssecondaire: prove the production OWL validation path is LIVE.What it proves
The production validator
Tests/OwlOntologyValidationTests.cs(a runtime validator invoked byOwlValidatorConfig.Apply, NOT an xUnit suite) had a silent false-pass:ValidateMultilingualAnnotations(L169-174) andValidateAIFMappings(L220-225) both early-returntrue("No concepts to validate — skipping") when_ontology.GetResourcesByType(SKOSVocabulary.Concept)returns empty.These tests drive the REAL production validator via reflection (its
_ontologyfield is private; the validation methods are public). They prove the fix delivered the actual production delta:Annotation_Validator_Inspects_Concepts_And_Passes_When_AnnotatedAnnotation_Validator_Fails_When_Concepts_Lack_Annotations_After_FixAif_Validator_Fails_When_Concepts_Have_No_Match_Mappings_After_FixAif_Validator_Passes_When_Concepts_Carry_ExactMatch_After_FixThe_Reader_The_Validators_Branch_On_Resolves_Concepts_NonEmptyGetResourcesByType(Concept).Count == 2(the predicate that gated the false-pass)The decisive tests are the two FAIL assertions: before the fix they would have been PASS (empty concepts → skip). After #481 the concepts resolve, the check actually runs, and it correctly reports the missing annotations/mappings. That contrast IS the proof the false-pass is dead.
Tests
223 pass / 0 fail / 5 skip (218 + 5). Deterministic, key-free, release-independent. No existing file modified.
AssetConverterConfig.cs/DatasetUpdaterRootConfig.cs/FallaciesLocalizationTests.csuntouched (reserved #444 β/γ po-2023).Relation to #481
This PR depends on #481's reader fix (the validator tests assert non-empty concept resolution, which only holds after the fix). Recommend merge order: #481 first, then this. If #481 is reverted, tests #2/#3 here flip red (correctly — they'd expose the returning false-pass).
🤖 Worker po-2024 — dispatch
4rkh1ssecondaire.