test(utility): #204 UtilityExtensions layout math + path gate (dispatch 4rkh1s tertiaire) - #483
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>
…ch 4rkh1s tertiaire)
NEW additive file: UtilityExtensionsLayoutTests.cs (+24 tests). Dispatch `4rkh1s` tertiaire —
cover a documented-fragile, deterministic, key-free area with ZERO existing coverage (grep
confirms no test references ToJaggedArray/UtilityExtensions/PathIsUrl).
(1) ToJaggedArray<T> — grid layout math. Pins rowLength = ceil(count/columns), the trailing
short row (remainder only, no padding — a padding regression would inject phantom blank cards
into final grid rows), row-major fill order (a transpose regression scrambles positions), and
the ToJaggedArray→Flatten inverse round-trip. This is the same class of geometry the coordinator
flagged ("count the geometry, not just the text").
(2) PathIsUrl — the asset-path gate. Pins that the relative paths CLAUDE.md documents as the
root cause of "white/empty cards" (../../Cards/..., ../, bare, Windows C:\, Unix /var/) are
NOT URLs (so the caller knows to rewrite them to absolute GitHub URLs), that http/https ARE
URLs, that null/empty/whitespace return false, and that whitespace-padded URLs still resolve.
Tests: 246 pass / 0 fail / 5 skip (223 + 24). Deterministic, key-free, release-independent.
No existing file modified. AssetConverterConfig.cs / DatasetUpdaterRootConfig.cs /
FallaciesLocalizationTests.cs untouched (reserved #444 β/γ po-2023).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
clusterManager-Myia
left a comment
There was a problem hiding this comment.
[NanoClaw]
PR scope
Production fix in OwlAdapter.cs (.Equals(string.URI) → .ToString() == .ToString() at 14 sites) + regression test updates + two new test files (OwlValidatorLivePathTests.cs — 6 tests proving the production validator path is live; UtilityExtensionsLayoutTests.cs — 24 tests for layout math + path gate). The PR body mentions 24 tests but there are actually 30 new test methods across the two new files (6 + 24), plus 8 renamed-but-repurposed tests in OwlAdapterRegressionTests.cs.
Production change: OwlAdapter.cs
The fix is mechanical and correct in intent: RDFResource.Equals(string) always returned false by type mismatch, so every fallback reader returned empty. Switching to .ToString() comparison sidesteps the type problem. Applied consistently across all 14 sites — GetAnnotationSubjects, GetAnnotationObjects, GetResourceAnnotations, GetLiteralAnnotations, CheckHasClass, HasAnnotation, CheckIsNarrowerConcept, GetExactMatchConcepts, GetCloseMatchConcepts, GetRelatedMatchConcepts, GetResourcesByType.
One concern: CheckIsNarrowerConcept's fix (line ~285) inverts the try/catch semantics. Before: try { return SKOSHelper... } catch { return annotation-scan }. After: if (SKOSHelper...) return true; catch { } return annotation-scan. The original try/return in the catch block is now gone — the catch is empty and the annotation scan always runs as the fallback. This is functionally fine (the annotation scan is the real fix), but it changes the control flow: before, if SKOSHelper threw, it immediately returned the annotation scan result; now it falls through to the annotation scan after the catch. Same outcome, different path. Worth noting this was intentional — not a bug.
Another concern: .ToString() comparison is fragile. If RDFResource.ToString() or GetIRI().ToString() ever changes format (e.g. adding angle brackets, changing URI encoding), these comparisons silently break again. The proper fix would be GetIRI().Equals(RDFResource.GetIRI()) to compare Uri to Uri, or accessing the underlying URI string uniformly. However, the ToString() approach works correctly today and the comprehensive regression tests will catch any future format change. This is an acceptable tradeoff given the test coverage.
Test quality: OwlAdapterRegressionTests.cs (modified)
The existing [BUG] characterization tests are correctly flipped from asserting broken behavior (empty/false) to asserting correct behavior (populated/true). The rename from BUG_* to descriptive names is clean. The CheckIsNarrowerConcept test now also asserts the negative case (Res("Unrelated") is not narrower of parent), which is a real improvement. CheckHasClass also adds the negative case. Good.
Minor note: the summary comment in the class doc still says "No existing file modified. Baseline additive." — but OwlAdapter.cs is modified in this PR. That line should be removed. This is cosmetic.
Test quality: OwlValidatorLivePathTests.cs (new)
This is the strongest addition. Using reflection to inject the private _ontology field into the production validator is clever — it proves the actual production code path is live, not just the adapter layer. The 6 tests are well-structured: pass-with-annotations, fail-without-annotations, fail-without-AIF-mappings, pass-with-AIF-mappings, and a contract guard on the specific reader the validators branch on.
One concern: OwlOntologyValidationTests is described as "NOT an xUnit suite — a runtime validator invoked by OwlValidatorConfig.Apply." The tests here instantiate it with new OwlOntologyValidationTests(config) and then inject via reflection. If the production class's constructor does non-trivial work (loading files, network, etc.), these tests could be flaky or environment-dependent. The claim is "deterministic, key-free, release-independent" which seems valid based on the code, but the constructor's side effects aren't visible in this diff. Worth verifying the constructor is safe.
Test quality: UtilityExtensionsLayoutTests.cs (new)
Well-structured. The ToJaggedArray tests cover:
- Exact division (6/3 = 2 full rows)
- Non-exact division with trailing short row (7/3 = [3,3,1])
- Theory parameterized row count across 6 cases including empty source
- Row-major order preservation (flatten round-trip)
- Inverse round-trip through
Flattenacross 6 different column lengths
The PathIsUrl tests cover http/https, relative paths, Windows paths, Unix paths, null, empty, whitespace-only, and whitespace-padded URLs.
Good edge cases: empty source (count=0), fewer items than columns (count=1, columns=3), null path, whitespace handling. These are the right boundary conditions.
Missing edge case for PathIsUrl: no test for FTP or other non-http schemes (ftp://, file://). If the implementation uses Uri.TryCreate with UriKind.Absolute, these would return true. If it explicitly checks for http/https only, they'd return false. Worth a test either way since the code path is a security gate (decides whether to rewrite asset paths).
Missing edge case for ToJaggedArray: columnLength=0 or columnLength=-1. The implementation presumably throws or returns empty, but untested. Minor since these are degenerate inputs unlikely in production.
Security
No secrets, no credentials, no connection strings found in the diff.
Verdict
Solid fix with good regression coverage. The .ToString() comparison approach is a pragmatic workaround rather than the ideal type-safe fix, but the tests guard it well. The OwlValidatorLivePathTests proving the production validation path is no longer a silent false-pass is particularly valuable. Recommend adding FTP/file scheme tests for PathIsUrl and consider the long-term fragility of .ToString() comparisons.
…α 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>
UtilityExtensions layout math + path gate (dispatch
4rkh1stertiaire)NEW additive file —
UtilityExtensionsLayoutTests.cs(+24 tests). Dispatch4rkh1stertiaire: cover a documented-fragile, deterministic, key-free area that had ZERO existing coverage (grep confirms no test referencesToJaggedArray/UtilityExtensions/PathIsUrl).What it covers
(1)
ToJaggedArray<T>— grid layout math (UtilityExtensions.cs:132). Pure function: list + columnLength → row-major jagged grid. This is the math behind PDF / mind-map column wrapping. Pinned:rowLength = ceil(count / columnLength)— exact-division and non-exact cases (Theory ×6).ToJaggedArray↔Flatteninverse round-trip — lossless for every column length 1…100.This is exactly the class of geometry assertion the coordinator flagged ("count the geometry, not just the text") for structured grids.
(2)
PathIsUrl— the asset-path gate (UtilityExtensions.cs:160). CLAUDE.md documents relative asset paths as the root cause of "white/empty cards" — this gate decides whether a path needs rewriting to an absolute GitHub URL. Pinned:../../Cards/...,../, bare, WindowsC:\, Unix/var/) → NOT URLs (so the caller rewrites them).Tests
246 pass / 0 fail / 5 skip (223 + 24). Deterministic, key-free, release-independent. No existing file modified.
AssetConverterConfig.cs/DatasetUpdaterRootConfig.cs/FallaciesLocalizationTests.csuntouched (reserved #444 β/γ po-2023).🤖 Worker po-2024 — dispatch
4rkh1stertiaire.