Big refactoring - compile patterns once and expand rows purely - #528
Merged
Conversation
CompiledPattern now carries each logical template as a parsed OWLClassExpression (or OWLAxiom for GCI) plus the placeholder slots discovered in it: a Map[varName, Set[OWLEntity]] for class/property variables and a Map[varName, OWLLiteral] for data variables. The no- bindings code path used today by Query/SPARQL is reused to produce these parsed values, so the placeholder convention is unchanged. Schema-level structural errors that the previous code surfaced as per-row parser exceptions now surface at compile time: - multi_clause on a full-axiom (GCI) template - sub_clauses on a logical multi_clause clause - multi_clause separator other than ' and ' or ' or ' ExpandedDOSDP still drives row expansion in this commit and uses the parsed expressions only via reference-equality lookups; the per-row Manchester parser is still invoked. Subsequent commits will introduce the Expansion module that consumes the new parsed values directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Expansion.expandRow takes a CompiledPattern, a RowBindings, and the ontology- and CLI-derived ExpansionContext, and returns either an ExpansionError or the row's axiom set. Substitution uses OWLObjectDuplicator against the placeholder entities and literals that PatternCompiler recorded on each ParsedPiece; the Manchester parser is not invoked at row time. AnnotationTranslation collects the pure annotation-rendering helpers that the row layer needs (translate, printAnnotation, permutation expansion). ExpandedRegexSub.substitute and expandBindings become pure; the only ZIO caller in Generate.renderRow drops its flatMap. Expansion is not yet wired into any subcommand; ExpandedDOSDP still drives row expansion. Subsequent commits switch callers and remove the redundant per-row Manchester parsing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Generate.renderPattern now builds an Expansion.ExpansionContext once per pattern and runs each row through Expansion.expandRow. The row-time helpers it used to keep on hand (resolveDefinedClass, resolveAxiomKinds, irisToLabels, resolveIrisToLabels, readableIdentifierForIRI) move into Expansion or disappear as duplicates. With no row-time caller left, ExpandedDOSDP shrinks to a pattern-template helper for Query/Terms/Docs. filledLogicalAxioms reads the parsed expressions off CompiledPattern instead of rerunning the Manchester parser; combineClauses is shared via CompiledClassExpression.combine. filledAnnotationAxioms is pure (no ZIO) and stays for SPARQL's annotation-query branch. AnnotationTranslation absorbs the duplicate translateAnnotations / printAnnotation / cartesianProduct code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three correctness gaps introduced when row-time Manchester parsing was moved to PatternCompiler: 1. Missing-binding placeholder leak. Expansion.substituteSinglePiece left the urn:dosdp: placeholder IRI in the duplicated expression when a row was missing a binding for one of the template's variables. The legacy PrintfText.replaced returned None and dropped the axiom. substituteSinglePiece now signals missing-required-binding via Option, the simple/multi/GCI/logical_axioms paths propagate it, and multi_clause survives clause-by-clause (matching the legacy join-the- non-empty-clauses behavior). 2. data_var in non-facet literal slots failed at compile time. PatternCompiler used to render with bindings=None, producing '$v' for every var. Class slots tolerated this (entity-name string), and facet slots tolerated it by datatype-coercion, but slots like `value %s` rejected the quoted-name literal. The compiler now renders class slots as '$name' and data-var slots as the explicit typed literal `"$name"^^<datatype>`, which the Manchester parser accepts in any literal-expecting context. PlaceholderLiteralPattern updates to match the new bare-`$name` lexical form, and collectLiterals walks OWLDataHasValue (the case that exposed the bug). data_vars in cardinality slots (`min N`, `max N`, ...) remain unsupported. 3. multi_clause sub_clauses on logical templates were rejected. compileMultiClause now assembles each top-level clause together with its sub_clauses into one Manchester string (via PrintfText) and parses it as a single piece, matching the legacy join-with-parent- separator semantics. withFallbackPlaceholders supplies '$name' bindings for any template var not declared in dosdp.vars / list_vars / data_vars / data_list_vars, so patterns referencing derived placeholder names (eg. __attribute) still compile. Tests: MissingBindingsTest covers six dropped-axiom cases (equivalentTo, subClassOf, disjointWith, GCI, logical_axioms entry, data_var literal slot) plus two multi_clause variants (partial miss, all miss). DataVarSlotTest covers the `value` slot and pins the existing facet regression. MultiClauseSubClausesTest pins the sub_clauses case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sub_clause drops parent on nested miss. compileMultiClause was flattening
each top-level PrintfClause + its sub_clauses into a single parsed piece,
so a missing binding in any nested sub_clause forced the entire group
to drop. Legacy PrintfText.replaceMultiClause kept the parent and
dropped only the missing sub-clause.
Compilation now preserves boundaries: CompiledPrintfClause holds the
main piece plus a list of CompiledSubExpression (each carrying its own
operator from its sep). Row-time expansion in Expansion drops a
clause-group only when its main piece misses bindings; surviving
sub-expressions combine independently.
Cardinality and DataOneOf literal walker. collectFromClassExpression
walked OWLDataSomeValuesFrom / OWLDataAllValuesFrom / OWLDataHasValue
but skipped OWLDataMinCardinality / OWLDataMaxCardinality /
OWLDataExactCardinality. A facet placeholder nested inside a cardinality
filler (eg. `'has_age' min 1 xsd:integer[>= %s]`) compiled but never
substituted at row time, leaking "$age". Same applied to DataOneOf
(`'has_age' some {%s}`) and to data ranges wrapped in
DataIntersectionOf / DataUnionOf / DataComplementOf.
collectFromDataRange now descends through all data ranges, and the class
expression walker hands off cardinality fillers to it. Tests pin both
the cardinality+facet and DataOneOf cases plus a missing-nested-sub_clause
and a missing-parent-with-bound-sub_clause scenario.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Single-clause nested sub_clause no longer requires a separator.
compileSubExpression called operatorFor(mc.sep.getOrElse("")) for every
nested MultiClausePrintf, so a one-clause sub_clause that omitted `sep`
failed compilation. The operator is only consulted when two or more
clauses must be joined; operatorFor now takes the clause count and
short-circuits to And (unused) for zero or one clause.
Placeholder-form parsed expression preserves nested operators.
CompiledMultiClassExpression.parsed flattened every nested piece and
combined them all with the top-level operator, so a parent `and` with a
nested `or` produced IntersectionOf(main, sub1, sub2) instead of
IntersectionOf(main, UnionOf(sub1, sub2)). Row-time expansion already
nested correctly, but ExpandedDOSDP.filledLogicalAxioms uses `parsed`
for the Query/Terms paths, so generated and queried axioms could
diverge. CompiledPrintfClause.parsedParts now contributes the main
piece plus each sub-expression collapsed via its own operator, and
CompiledSubExpression.parsed combines its clauses with its own
operator — mirroring the row-time structure exactly.
Tests cover the no-sep single-clause sub_clause and a parent-and /
nested-or pattern asserting both the row-time and placeholder-form
axioms nest a union inside the intersection.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous operatorFor short-circuited to And whenever clauseCount
was <= 1, discarding an explicit sep. That is wrong: a single
PrintfClause expands to multiple expressions at row time when its
filler is a list_var, and the legacy renderer joined those expansions
with the declared separator. A one-clause multi_clause with sep ' or '
and a piped list filler now correctly produces a union instead of an
intersection.
operatorFor now honors an explicit sep (' and ' / ' or ', error
otherwise) regardless of clause count, and only defaults to And when
sep is absent and there is at most one clause. Absent sep with two or
more clauses is now an explicit compile error rather than a downstream
Manchester parse failure.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Snapshots each fixture's full Set[OWLAxiom] and asserts no urn:dosdp: placeholder ever leaks into rendered output. Goldens are generated from the pre-refactor faddaf8 commit via its generate CLI, so a green test means the refactor preserved behavior for that fixture. Comparison is set-equality, since functional-syntax serialization order is not stable enough for byte diffs. Retrofits existing generate-path tests (Override, BlankLine, MissingValues, InternalVars, Permutation) and adds five new YAML+TSV fixtures for shapes that only inline-DOSDP tests covered: nested multi_clause with mixed and/or operators, data_var in value/facet/DataOneOf/facet-in-cardinality slots, the four axiom kinds in one pattern, list_var folding into intersection and union, and OBO annotations alongside axiom and sub annotations. Ten of eleven fixtures preserved behavior against faddaf8. The eleventh, multi_clause_sub, intentionally diverges: legacy flat-string parsing let an "or" inside a sub_clause escape its parent "and"; the refactor preserves the nesting (commit b24cee4). The golden captures the corrected output and ComplexStructureGoldenTest documents the change inline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds Jena structural-compare for SPARQL, byte-text compare for Markdown, and set-of-lines compare for unordered IRI dumps; reuses the existing OFN axiom-set compare where applicable. Each subcommand is exercised through its public surface against goldens generated from the pre-refactor faddaf8 build. Coverage is currently narrower than generate. Query goldens cover only two fixtures (OverrideTest, test_blank_lines) because faddaf8's placeholder-form Manchester parser errors on most complex fixtures (GCI, multi_clause, annotated logical axioms, list_vars in expressions, data_vars). That fragility is precisely what the refactor's new compile-time placeholder model was meant to address; broader query coverage will need refactoring-3-anchored goldens in a follow-up. Prototype tests are deliberately omitted: the harness immediately surfaced a regression. Prototype.axiomsFor builds synthetic fillers from dosdp.vars, whose values are class-label range expressions like 'thing'. The faddaf8 path string-substituted these and let the Manchester entity checker resolve them via the classes map; the refactor's row-time OWLObjectDuplicator substitution expects IRIs, so the run errors with "Binding for '<var>' did not resolve to an IRI". SubcommandGoldenTest documents the cause inline so the tests can be re-enabled once the prototype path is restored. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prototype.axiomsFor synthesizes fillers from dosdp.vars, whose values are class-label range expressions like 'thing'. Faddaf8's renderPattern substituted these strings into templates and let the Manchester entity checker resolve them via the classes map at parse time. The refactor moved entity resolution to compile time, so row-time substitution started rejecting labels with "Binding for '<var>' did not resolve to an IRI" and the prototype subcommand stopped producing output for any pattern whose class vars were ranged over labels. Expansion.expandRow now pre-resolves class-var and list-var bindings through Prefixes.nameOrVariableToIRI plus the pattern's classes map when assembling logicalBindings — but only for logicalBindings. The annotation bindings are built separately from the unresolved values, so annotation text still renders the literal label form a pattern author wrote (e.g. "part of 'thing'"), matching faddaf8 byte-for-byte. Values that already parse as IRIs or CURIEs pass through unchanged, so normal generate-path rows are unaffected. SubcommandGoldenTest re-enables the two prototype tests deferred in the previous commit; both now pass against the faddaf8 baseline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
OWLAxiom.getSignature only returns named entities (classes, properties, individuals, datatypes), so a urn:dosdp: IRI used as an AnnotationAssertion subject — or as an IRI-valued annotation — slips past a signature-only walk. The OWL API's OWLObjectComponentCollector also doesn't reliably expose those bare IRIs in this position, so the helper now scans the canonical functional-syntax serialization, which enumerates every IRI by construction. The literal walker switches to the same approach for consistency. MissingBindingsTest and MultiClauseSubClausesTest each carried a private placeholderIRIs helper with the same getSignature bug; they now go through Harness.assertNoPlaceholderIRIs so future fixes propagate from one place. New HarnessTest pins the three placeholder-IRI cases (subject, IRI-valued annotation, entity signature) plus the literal case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add AuditByShapeTest covering the gaps in the template-kind × var-kind × annotations matrix: axiom-annotated equivalentTo/disjointWith/GCI, missing-binding negatives on OBO / custom / permutations / override annotation paths, and missing-binding negatives on the three non-facet data_var Manchester slots. Pin the known cardinality-position regression in PatternCompilerTest: a data_var in `min %s` fails at compile time because the typed-literal placeholder doesn't parse where Manchester expects a bare integer. Broaden SubcommandGoldenTest query coverage to axiom_kinds, list_var_logical, and annotated_axioms (refactoring-3 anchored). Harness.assertSPARQLMatchesGolden now normalizes the SPARQL generator's UUID intermediates so goldens survive re-randomization. data_var_slots and multi_clause_sub stay deferred — documented inline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds RowBindings.placeholder(dosdp) + RowBindings.placeholderRow that bind every declared variable to its own urn:dosdp: IRI (class/list) or $<name> literal (data) so Expansion.expandRow becomes a noop substitution producing the same placeholder-form axioms that ExpandedDOSDP.filledLogicalAxioms emits today. This is the seam Query and Terms will switch onto in 3.0.2. Prefixes.idToIRI now recognizes urn:-scheme IRIs as absolute so placeholder bindings round-trip; without this the CURIE regex eagerly matches urn: and fails for lack of a prefix mapping. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ExpandedDOSDP.placeholderAxioms(kinds) replaces filledLogicalAxioms / filledAnnotationAxioms by delegating to Expansion.expandRow with a synthetic placeholder row, so Query, Terms, and the existing pure row expansion all share one OWL-assembly implementation. SPARQL.queryFor hoists the logical-axiom set so selectFor and triplesFor share one expansion per query. RowBindings.placeholder(compiled) walks the compiled pattern's parsed pieces to bind every class slot, including fallback placeholders the compiler inserted for template vars that aren't declared on the DOSDP (otherwise allSlotsBound would drop the axiom). Declared vars / list_vars are unioned in so annotations on vars that have no logical-piece appearance still resolve their bindings. DOSDP.literalPlaceholder centralizes the `$<name>` lexical form previously open-coded in three sites. Prefixes.idToIRI now recognizes urn:-scheme IRIs so placeholder bindings round-trip through it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
RowBindings.placeholder previously folded every declared list_var and data_list_var into the single-value maps, leaving listVarBindings and dataListBindings empty. NormalizedListAnnotation only consults the multi-value maps, so placeholder expansion silently dropped every list-style annotation (exact_synonym, xref, custom value-list annotations) and the resulting SPARQL annotation queries omitted those triples entirely. Bind declared list_vars to MultiValue(Set(urn:dosdp:name)) and data_list_vars to MultiValue(Set($name)). Logical-axiom expansion stays correct because expandClauseWithMultiValue iterates the one placeholder value into a single-value substitution, producing the same expression shape as before. Add a list_annotations fixture covering exact_synonym + xrefs and a custom ListAnnotation, pinned through both PlaceholderRowTest and a new query_all SubcommandGoldenTest entry that exercises AllAxioms so the emitted annotation triples are part of the snapshot. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
triplesForAnnotationValue previously treated any annotation-value literal as opaque text and scanned for embedded urn:dosdp: tokens. A literal whose entire lexical form is $name (the shape that a data_list_var filler in a list-style annotation now emits, since the prior commit fixed placeholder bindings to carry MultiValue placeholders) fell through to a regex match against the literal text itself, producing FILTER(REGEX(?v, "\Q$name\E")) — a constraint no real ontology value can satisfy. Match the entire-literal-is-placeholder case via a new DOSDPLiteralPlaceholder regex (^\$(\w+)$) and emit ?name as a SPARQL variable, mirroring how DOSDPVariable handles a whole-IRI placeholder. The embedded-$name case (data_var inside printf text) is unchanged — it stays a pre-existing limitation, separate from the list-annotation fix the prior commit introduced. Regenerate list_annotations.query_all.golden.rq so the ?defined_class oboInOwl:hasExactSynonym ?synonym triple and the seeAlso triple are now functional rather than locking in the broken FILTER(REGEX(...)) shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The new literal-placeholder branch in triplesForAnnotationValue emitted a bare ?var with no term-kind constraint, so a query like ?defined_class rdfs:seeAlso ?synonym . would match IRI-valued seeAlso assertions in addition to the literal-valued ones the list-style annotation actually produces — a false-positive class of matches for annotation/all queries. Mirror the IRI branch (FILTER(isIRI(?var))) and add FILTER(isLiteral(?var)) so the captured variable only binds literal terms, matching the OWL-level constraint that NormalizedListAnnotation emits literal annotation values. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 3.0.3. The four review rounds during Phase 2b originated in discovering literal-bearing OWL constructs incrementally - DataHasValue, then cardinalities, then DataOneOf - instead of enumerating the set once from the OWL API. The literal walker is closed-form, but nothing in the codebase asserted that fact. LiteralWalkerAuditTest exercises every ClassExpressionType and DataRangeType value the OWL API exposes: builds a sample expression of each type containing a known placeholder literal, runs the walker, and asserts whether the placeholder is recovered against the per-type expectation. If a new enum value is ever added (an OWL API upgrade or spec extension), the first assertion fires with "test missing entry for X," forcing a deliberate decision about whether the new type carries literals and needs walker support. Three additional tests pin: nested-class-expression descent via getNestedClassExpressions reaching a data restriction, the same descent through an OWLAxiom, and one-level axiom-annotation literal recovery. Promote collectLiterals / collectFromClassExpression / collectFromDataRange from private to private[dosdp] so the audit can call them. Document the audit's scope in a docstring on collectLiterals, including the deliberate non-coverage of sub-annotations and non-class-expression axiom subjects/values (the inline Manchester parser used at compile time produces neither). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Expansion.placeholderAxioms(compiled, kinds) replaces the method that lived on ExpandedDOSDP, keeping the synthetic-placeholder-row expansion next to expandRow itself. VarRangeExpressions houses the var / list_var Manchester-range parser used by SPARQL and Docs. ExpandedRegexSub moves next to its Binding consumers in Bindings.scala. ExpandedDOSDP.scala is gone. SPARQL.queryFor / triplesFor, Terms.run, Query.makeProcessedQuery, and DocsMarkdown.markdown now take a CompiledPattern directly. The per-call ExpandedDOSDP(dosdp, prefixes, compiled) wrapper is gone — CompiledPattern already carries source and prefixes, so the wrapper was just shuffling the same three values through one more constructor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Names its single responsibility (pure Markdown rendering of a compiled pattern plus example fillers) and clarifies that ontology loading and file I/O stay in the cli.Docs runner, matching the docstring style on the other modules introduced by the refactor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Recover the original variable name in `describePiece` via a reverse `Map[IRI, String]` lookup. `DOSDP.variableToIRI` normalizes spaces to underscores, so stripping the `urn:dosdp:` prefix records the slot as `cell_type` when the declared variable (and TSV column) is `cell type` — `allSlotsBound` then misses the binding and drops every axiom that references the variable. Make `compileClassExpressionBody` return `Option[CompiledClassExpression]` and emit `None` when a class-expression template has neither `text` nor `multi_clause`. `compileLogicalAxiom` propagates the `Option` and the caller flattens. A `subClassOf` / `equivalentTo` / `disjointWith` / non-GCI `logical_axioms` entry with an empty body previously compiled to `owl:Thing` and surfaced as `EquivalentClasses(defined_class, owl:Thing)` (or similar) for every row. Add `CompiledPatternCorrectnessTest` with seven cases pinning both invariants (var-name with a space across equivalentTo/subClassOf/ logical_axioms, and empty bodies on each of the four logical positions). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add cases the suite did not previously exercise:
- empty `multi_clause` containers (clauses=Some(Nil), clauses=None) on
equivalentTo
- annotations-only equivalentTo (annotations present but no text or
multi_clause) — no spurious `defined_class ≡ owl:Thing`
- empty `logical_axioms` entries for axiom_type SubClassOf and
DisjointWith (only EquivalentTo was covered)
- var names containing characters other than a space (hyphen, dot,
multiple consecutive spaces) — pins the reverse-lookup contract
in `describePiece` for any name that survives `processedVariable`
unchanged or under a different rewrite
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
compileAxiomTemplate and the GCI branch of compileLogicalAxiom were still failing loudly via parseAxiomText for a top-level GCI block or a logical_axioms entry with axiom_type GCI that has no text. The previous 530f2b0 fix restored the legacy silent-drop only for the class-expression types; this brings GCI into symmetry. Empty GCI templates now compile to None and emit no axiom, matching the legacy PrintfText.replaced(None) silent-drop behavior. Add three regression tests pinning the new behavior: empty top-level GCI body, empty GCI logical_axioms entry, and annotations-only top-level GCI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR performs a major refactor of DOSDP pattern expansion by compiling/parsing Manchester templates once into placeholder-bearing OWL objects, then performing per-row substitution purely (via OWLObjectDuplicator). It removes the old ExpandedDOSDP row-expansion helper and adds an extensive “behavior preservation” test harness (goldens + invariants) to guard against regressions across CLI subcommands (generate, query, terms, docs).
Changes:
- Replace per-row Manchester parsing with a compiled placeholder OWL representation (
CompiledPattern) plus pure row expansion (Expansion). - Update
SPARQL,Query,Terms,Docs(and related plumbing) to use placeholder-form axioms produced byExpansion.placeholderAxioms. - Add a golden/invariant test harness and many new fixtures to lock behavior and detect placeholder leaks.
Reviewed changes
Copilot reviewed 77 out of 77 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/main/scala/org/monarchinitiative/dosdp/Expansion.scala | New pure row-expansion engine returning Either[ExpansionError, Set[OWLAxiom]], plus placeholder-row expansion for query/terms. |
| src/main/scala/org/monarchinitiative/dosdp/CompiledPattern.scala | Refactors compilation to parse Manchester once into placeholder-bearing OWL, adds slot/literal walkers and multi_clause compilation model. |
| src/main/scala/org/monarchinitiative/dosdp/AnnotationTranslation.scala | New pure translation layer for normalized annotations, including permutations. |
| src/main/scala/org/monarchinitiative/dosdp/VarRangeExpressions.scala | New module to parse vars / list_vars range expressions for SPARQL restrictions and docs. |
| src/main/scala/org/monarchinitiative/dosdp/SPARQL.scala | Switches SPARQL generation to operate on CompiledPattern + placeholder axioms; adds special handling for $name literal placeholders. |
| src/main/scala/org/monarchinitiative/dosdp/Prefixes.scala | Treat URN IRIs (notably urn:dosdp:*) as absolute IRIs during CURIE/IRI resolution. |
| src/main/scala/org/monarchinitiative/dosdp/DOSDP.scala | Adds literalPlaceholder ($<name>) helper used for compile-time data-var placeholders. |
| src/main/scala/org/monarchinitiative/dosdp/DocsMarkdown.scala | Updates Markdown rendering to consume CompiledPattern and VarRangeExpressions (no ExpandedDOSDP). |
| src/main/scala/org/monarchinitiative/dosdp/Bindings.scala | Moves ExpandedRegexSub here and makes substitution pure (no ZIO/logging). |
| src/main/scala/org/monarchinitiative/dosdp/cli/Generate.scala | Replaces ExpandedDOSDP row rendering with Expansion.expandRow; adds placeholder binding helpers for query/terms/docs. |
| src/main/scala/org/monarchinitiative/dosdp/cli/Query.scala | Uses SPARQL.queryFor(compiled, ...) (no ExpandedDOSDP). |
| src/main/scala/org/monarchinitiative/dosdp/cli/Terms.scala | Uses Expansion.placeholderAxioms(compiled, LogicalAxioms) to seed “pattern terms”. |
| src/main/scala/org/monarchinitiative/dosdp/cli/Docs.scala | Uses DocsMarkdown.markdown(compiled, ...) (no ExpandedDOSDP). |
| src/main/scala/org/monarchinitiative/dosdp/ExpandedDOSDP.scala | Deleted (old row-time parsing/expansion path removed). |
| src/test/scala/org/monarchinitiative/dosdp/UnionQueryTest.scala | Updates SPARQL query generation call site to accept CompiledPattern. |
| src/test/scala/org/monarchinitiative/dosdp/QueryGeneratorTest.scala | Updates SPARQL query generation call site to accept CompiledPattern. |
| src/test/scala/org/monarchinitiative/dosdp/QueryGeneratorRegexTest.scala | Updates SPARQL triple generation call site to accept CompiledPattern. |
| src/test/scala/org/monarchinitiative/dosdp/RegexTest.scala | Updates regex substitution tests to the new pure ExpandedRegexSub API. |
| src/test/scala/org/monarchinitiative/dosdp/SubcommandGoldenTest.scala | New: golden snapshots for query/terms/prototype/docs subcommands. |
| src/test/scala/org/monarchinitiative/dosdp/PlaceholderRowTest.scala | New: goldens for placeholder-row expansion (Expansion.placeholderAxioms). |
| src/test/scala/org/monarchinitiative/dosdp/ComplexStructureGoldenTest.scala | New: golden snapshots for complex pattern structures to validate behavior preservation. |
| src/test/scala/org/monarchinitiative/dosdp/Harness.scala | New: shared golden + placeholder-leak assertion utilities. |
| src/test/scala/org/monarchinitiative/dosdp/HarnessTest.scala | New: sanity tests for Harness placeholder detection. |
| src/test/scala/org/monarchinitiative/dosdp/LiteralWalkerAuditTest.scala | New: audit coverage for literal collection across OWL API enum types. |
| src/test/scala/org/monarchinitiative/dosdp/AuditByShapeTest.scala | New: fills matrix gaps for missing-binding and annotation behavior. |
| src/test/scala/org/monarchinitiative/dosdp/MissingBindingsTest.scala | New: regression tests ensuring missing bindings drop axioms/annotations (no placeholder leaks). |
| src/test/scala/org/monarchinitiative/dosdp/DataVarSlotTest.scala | New: tests for data_var substitution in multiple Manchester literal slots. |
| src/test/scala/org/monarchinitiative/dosdp/CompiledPatternCorrectnessTest.scala | New: invariants around var-name normalization and empty logical blocks not emitting axioms. |
| src/test/scala/org/monarchinitiative/dosdp/MultiClauseSubClausesTest.scala | New: coverage for nested sub_clauses behavior in logical multi_clause templates. |
| src/test/scala/org/monarchinitiative/dosdp/MultiClausePrintfTest.scala | Adds test coverage for single-clause multi_clause with or folding list_var expansions into a union. |
| src/test/scala/org/monarchinitiative/dosdp/PatternCompilerTest.scala | Adds explicit test pinning known non-support of data_var in cardinality slot. |
| src/test/scala/org/monarchinitiative/dosdp/PermutationTest.scala | Adds goldens and placeholder-leak assertions to permutation-related generation tests. |
| src/test/scala/org/monarchinitiative/dosdp/OverrideTest.scala | Adds goldens and placeholder-leak assertions to override-related generation tests. |
| src/test/scala/org/monarchinitiative/dosdp/MissingValuesTest.scala | Adds goldens and placeholder-leak assertions for missing-values behavior. |
| src/test/scala/org/monarchinitiative/dosdp/InternalVarsTest.scala | Adds placeholder-leak assertions and goldens to internal-vars fixtures. |
| src/test/scala/org/monarchinitiative/dosdp/BlankLineTest.scala | Adds golden + placeholder-leak assertions for blank-line fixture behavior. |
| src/test/resources/org/monarchinitiative/dosdp/test_blank_lines.query.golden.rq | New query golden for blank-lines fixture. |
| src/test/resources/org/monarchinitiative/dosdp/test_blank_lines.golden.ofn | New OFN golden for blank-lines fixture output. |
| src/test/resources/org/monarchinitiative/dosdp/OverrideTest.query.golden.rq | New query golden for OverrideTest fixture. |
| src/test/resources/org/monarchinitiative/dosdp/OverrideTest.golden.ofn | New generate golden for OverrideTest fixture output. |
| src/test/resources/org/monarchinitiative/dosdp/missing_values_test.golden.ofn | New generate golden for missing-values fixture output. |
| src/test/resources/org/monarchinitiative/dosdp/internal_vars_test.golden.ofn | New generate golden for internal-vars fixture output. |
| src/test/resources/org/monarchinitiative/dosdp/permutation_test.placeholder.golden.ofn | New placeholder-row OFN golden for permutation fixture. |
| src/test/resources/org/monarchinitiative/dosdp/permutation_test.golden.ofn | New generate golden for permutation fixture output. |
| src/test/resources/org/monarchinitiative/dosdp/permutation_cartesian.tsv | New fixture input TSV for cartesian permutation test. |
| src/test/resources/org/monarchinitiative/dosdp/permutation_cartesian.golden.ofn | New generate golden for cartesian permutation output. |
| src/test/resources/org/monarchinitiative/dosdp/multi_clause_sub.yaml | New fixture pattern for nested sub_clauses logical multi_clause. |
| src/test/resources/org/monarchinitiative/dosdp/multi_clause_sub.tsv | New fixture fillers for multi_clause_sub. |
| src/test/resources/org/monarchinitiative/dosdp/multi_clause_sub.terms.golden.txt | New golden for terms output for multi_clause_sub. |
| src/test/resources/org/monarchinitiative/dosdp/multi_clause_sub.placeholder.golden.ofn | New placeholder-row OFN golden for multi_clause_sub. |
| src/test/resources/org/monarchinitiative/dosdp/multi_clause_sub.golden.ofn | New generate golden for multi_clause_sub output. |
| src/test/resources/org/monarchinitiative/dosdp/list_var_logical.yaml | New fixture pattern for list_var folding into logical axioms. |
| src/test/resources/org/monarchinitiative/dosdp/list_var_logical.tsv | New fixture fillers for list_var_logical. |
| src/test/resources/org/monarchinitiative/dosdp/list_var_logical.query.golden.rq | New query golden for list_var_logical. |
| src/test/resources/org/monarchinitiative/dosdp/list_var_logical.placeholder.golden.ofn | New placeholder-row OFN golden for list_var_logical. |
| src/test/resources/org/monarchinitiative/dosdp/list_var_logical.golden.ofn | New generate golden for list_var_logical output. |
| src/test/resources/org/monarchinitiative/dosdp/list_annotations.yaml | New fixture for list-style annotations and data_list var binding. |
| src/test/resources/org/monarchinitiative/dosdp/list_annotations.query_all.golden.rq | New “all axioms” query golden for list_annotations. |
| src/test/resources/org/monarchinitiative/dosdp/list_annotations.placeholder.golden.ofn | New placeholder-row OFN golden for list_annotations. |
| src/test/resources/org/monarchinitiative/dosdp/data_var_slots.yaml | New fixture pattern covering multiple data_var Manchester slots. |
| src/test/resources/org/monarchinitiative/dosdp/data_var_slots.tsv | New fixture fillers for data_var_slots. |
| src/test/resources/org/monarchinitiative/dosdp/data_var_slots.placeholder.golden.ofn | New placeholder-row OFN golden for data_var_slots. |
| src/test/resources/org/monarchinitiative/dosdp/data_var_slots.golden.ofn | New generate golden for data_var_slots output. |
| src/test/resources/org/monarchinitiative/dosdp/axiom_kinds.yaml | New fixture pattern combining multiple axiom kinds. |
| src/test/resources/org/monarchinitiative/dosdp/axiom_kinds.tsv | New fixture fillers for axiom_kinds. |
| src/test/resources/org/monarchinitiative/dosdp/axiom_kinds.terms.golden.txt | New terms golden for axiom_kinds. |
| src/test/resources/org/monarchinitiative/dosdp/axiom_kinds.query.golden.rq | New query golden for axiom_kinds. |
| src/test/resources/org/monarchinitiative/dosdp/axiom_kinds.prototype.golden.ofn | New prototype golden for axiom_kinds. |
| src/test/resources/org/monarchinitiative/dosdp/axiom_kinds.placeholder.golden.ofn | New placeholder-row OFN golden for axiom_kinds. |
| src/test/resources/org/monarchinitiative/dosdp/axiom_kinds.golden.ofn | New generate golden for axiom_kinds output. |
| src/test/resources/org/monarchinitiative/dosdp/annotated_axioms.yaml | New fixture pattern for annotated logical axioms + sub-annotations. |
| src/test/resources/org/monarchinitiative/dosdp/annotated_axioms.tsv | New fixture fillers for annotated_axioms. |
| src/test/resources/org/monarchinitiative/dosdp/annotated_axioms.query.golden.rq | New query golden for annotated_axioms. |
| src/test/resources/org/monarchinitiative/dosdp/annotated_axioms.prototype.golden.ofn | New prototype golden for annotated_axioms. |
| src/test/resources/org/monarchinitiative/dosdp/annotated_axioms.placeholder.golden.ofn | New placeholder-row OFN golden for annotated_axioms. |
| src/test/resources/org/monarchinitiative/dosdp/annotated_axioms.golden.ofn | New generate golden for annotated_axioms output. |
| src/test/resources/org/monarchinitiative/dosdp/annotated_axioms.docs.golden.md | New docs Markdown golden for annotated_axioms. |
Comments suppressed due to low confidence (1)
src/main/scala/org/monarchinitiative/dosdp/SPARQL.scala:148
DOSDPLiteralPlaceholderis currently\A\$(\w+)\z, which won’t match placeholder lexical forms for data/data_list vars whose names include non-word characters (e.g. hyphens, dots, spaces). In that case the code falls back to the regex-match path and will never bind the literal variable. Suggest matching the same placeholder shape the compiler emits (^\$(.+)$) and then mapping back to a legal SPARQL variable name if needed.
private val DOSDPLiteralPlaceholder = """\A\$(\w+)\z""".r
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Now that validateVariableNames restricts variable names to [A-Za-z0-9_]+, DOSDP.processedVariable is a no-op for every accepted name and the URN prefix strip recovers the original. Drop the reverse-lookup map and the variableNames argument from describePiece; the validator owns the contract, and a fallback in the parser would invite silent disagreement if the validator ever changed. Add four PatternCompilerTest cases that walk the deeper positions of the variableNames collector: a permutation var, an internal_vars input reference, a name nested inside a multi_clause sub_clause, and an IRIValueAnnotation var. The shallow positions were already covered; these lock in that the walker descends through every variable-bearing shape in the YAML. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OWL values; per-row expansion substitutes via
OWLObjectDuplicatorandreturns
Either[ExpansionError, Set[OWLAxiom]]— no ZIO, no Logging inthe row layer.
query/terms/docsnow obtain their placeholder-form axioms bycalling
Expansion.expandRowwith a synthetic placeholder row. Theparallel
ExpandedDOSDP.filledLogicalAxioms/filledAnnotationAxiomspath is deleted, removing the class of "the two paths disagree" bugs.
ExpandedDOSDP.scalais gone; its surviving pieces moved to focusedmodules (
Expansion,CompiledPattern,AnnotationCompiler,AnnotationTranslation,NormalizedAnnotation,VarRangeExpressions,DocsMarkdown).pre-refactor commit
faddaf8, placeholder-leak invariants, audit-by-shapetable) is in place and runs as part of
sbt test.