feat(graph-ir): fallible GraphSink protocol, quad capability, statement lifecycle + Turtle ParserOptions conformance - #1552
feat(graph-ir): fallible GraphSink protocol, quad capability, statement lifecycle + Turtle ParserOptions conformance#1552aaj3f wants to merge 14 commits into
Conversation
…ycle The sink protocol had no error channel: every emit method returned `()`, so a sink that writes somewhere could not report failure and a producer could not stop. `convert big.ttl | head -5` would parse to EOF against a dead pipe. It also had no trait-level flush, and no way for a producer to tell a sink when a statement ended. Protocol changes (fluree-graph-ir/src/sink.rs): - `emit_triple` / `emit_list_item` / `emit_reified_triple` return `Result<(), SinkError>`; `SinkError` carries an `io::Error` or a rejection message and answers `is_broken_pipe()` so a CLI can exit quietly on a closed downstream. `term_*` stay infallible — they only intern. - `supports_quads()` + `emit_quad()`, mirroring the `supports_reified_triples` capability probe. The default body errors rather than no-ops: no producer emits quads yet, so this backstop is what stops a future one from silently folding graph names into the default graph. (`emit_reified_triple`'s default stays a no-op — every producer of those events already refuses up-front on the probe, so its default is unreachable, not a fallback.) - `end_statement()`, called by the Turtle parser at each statement terminator, and only for statements that parsed and emitted cleanly. Two consumers: statement-scoped term lifetime (literal ids are per-occurrence and are the O(document) growth term), and the statement-scoped output buffering `--continue-on-error` needs, since triples are emitted during descent before the terminating `.` proves the statement valid. - `finish()`, the protocol flush. Producers do NOT call it: one sink can be fed by many producer calls (a chunk each on the bulk-import path). Because `finish()` now means "flush", the sinks that also produce something expose that separately as a consuming method — `GraphCollectorSink::into_graph`, `FlakeSink::into_flakes`, `ImportSink::into_parts`. Leaving them named `finish()` would have let a concrete-typed caller silently skip the protocol finish, which is exactly the trap a writer sink cannot survive. Deferred-error semantics in FlakeSink/ImportSink are unchanged: `emit_*` still records the first invariant violation and keeps going, and the consuming method is still where it becomes a hard error. The Turtle parser propagates sink errors immediately, and preserves them as `TurtleError::Sink` rather than flattening them into a generic parse error. The JSON-LD adapter does the same via `AdapterError::Sink`; its module docs now say `@graph` contents are dropped rather than flattened, which is what the code does.
`GraphCollectorSink` kept one live term-table entry per literal *occurrence* for the length of the document: literals are minted per occurrence and never deduplicated, unlike IRI and blank ids, which producers cache and reuse. On a large Turtle file that means every literal is held twice — once in the term table, once in the graph. `end_statement` (landed with the protocol) marks the point where the statement's literal ids are provably dead, so the sink now reuses their slots. The high-water mark becomes the widest single statement instead of the whole document; a 1000-statement document of two literals each leaves 4 term slots behind rather than 2002. Soundness rests on two facts, both pinned by tests: `emit_*` clones a term into the graph before returning, so overwriting a slot afterwards cannot reach a triple already collected; and the Turtle parser caches only IRI and prefixed-name ids across statements, never literal ids. Producers that do not delimit statements — the JSON-LD adapter, and any direct caller of the sink API — never call `end_statement` and so keep today's allocation behavior exactly.
The parser canonicalizes in three places that are correct for ingest and
wrong for conversion:
- an object-position collection emits flat indexed list items, so the three
triples W3C defines for `( ex:o )` arrive as one event;
- an object-position `()` emits nothing at all — a silently lost triple,
since `()` denotes `rdf:nil`;
- `Integer` and `Double` tokens are parsed to native values, so `+1`, `01`,
`1e0` and `1.0E0` are unrepresentable. (`Decimal` and i64-overflowing
integers already keep their lexical form.)
Subject-position collections, meanwhile, have always emitted a proper
spine, so the two positions disagreed with each other.
`ParserOptions { collections, numerics }` gets both shapes out of one
grammar. `ParserOptions::default()` is exactly today's behavior in every
field, so every existing caller is untouched; `ParserOptions::conformant()`
is the faithful-RDF preset. New entry points `parse_with_options` and
`parse_with_prefixes_base_options`; the existing `parse` /
`parse_with_prefixes_base` now delegate with defaults.
`CollectionStyle::Spine` needs no new collection code: it drops the
object-list special cases so `( … )` and `()` fall through to the ordinary
object path, which already builds a spine and already resolves `()` to
`rdf:nil`. One code path now serves both positions. A consequence worth
naming: a spine collection has a single object term (the head), so the RDF
1.2 annotation-on-collection deferral — which exists only because indexed
items leave nothing to reify — does not apply in this mode.
`NumericStyle::PreserveLexical` routes numeric tokens through the
typed-string lane that `Decimal` and `IntegerOverflow` already use, rather
than adding a second representation to the IR. Nothing in fluree-graph-ir
changes, and the ingest path keeps hitting the identical branch it hits
today.
Tests ship with the feature: W3C-shaped spine assertions that walk the
rdf:first/rdf:rest chain rather than counting triples, empty collections in
both positions, the seven lexical forms, and — as the regression guard for
"existing callers are unchanged" — a triple-for-triple pin of default-mode
output across every construct the options touch. The pre-existing
`test_collection` / `test_empty_collection` / `test_integer_literal`
assertions are unmodified and still pass.
The recycling failure mode is aliasing, not arithmetic: a wrong answer is a perfectly well-formed graph with one term silently substituted for another. Per-triple expectations written by the author of the recycling are unlikely to anticipate that, so this asserts against an oracle instead — the same document parsed into a sink that recycles and into one whose `end_statement` is swallowed by a forwarding wrapper, i.e. today's behavior. Any TermId outliving its statement while still referenced diverges. The corpus is chosen to reuse slots at every width: statements that emit during descent (nested property lists, collections at two levels, subject-position spines), literal counts that shrink and grow across boundaries, repeated identical literals as aliasing bait, and blank-node labels shaped like the anonymous mint. Run in default and spine modes, plus the bulk-import shape of one sink fed by many parses. Mutation-checked rather than assumed: recycling IRI slots — which the parser caches across statements — fails both hostile-document cases. Notably the single-sink and failed-statement cases do NOT catch that mutation, which is recorded in the module docs so nobody trims the corpus believing they are redundant. The failed-statement case pins an observable protocol property rather than recycling: a statement that errors after emitting never reaches `end_statement`, and the sink keeps the half-statement's triples. There is no rollback hook, which is precisely why `--continue-on-error` will need the statement-scoped buffering `end_statement` exists to enable. Approach and hostile corpus came from the adversarial review of this PR.
`GraphCollectorSink` minted anonymous blank nodes as `b{N}`, which is
exactly the shape a document writes by hand. A file containing `_:b1` and
any anonymous node therefore produced ONE node where RDF says two, silently
merging user data into a parser-minted node.
Reproduced: `_:b1 ex:p "user-one" .` plus a spine collection in one
document leaves `_:b1` owning three triples instead of one.
Pre-existing via `[ … ]` and reifiers, but `CollectionStyle::Spine` makes it
systematic — every collection mints spine nodes — so it has to be fixed
here rather than filed.
The fix is the one `FlakeSink` and `ImportSink` already carry, with the same
rationale recorded in their comments: mint `-b{N}`. Turtle's
BLANK_NODE_LABEL must start with PN_CHARS_U | [0-9], so `_:-b1` can never
lex and the minted namespace is unreachable from any input, while '-' stays
legal medially so the label still serializes.
Two tests, both mutation-verified against the `b{N}` mint: a unit test that
the minted label cannot lex as a user label, and a Turtle-level test that
`_:b1`/`_:b2` alongside a spine collection and a `[ … ]` keep every identity
distinct, in both option modes. Deliberately identity-counting rather than
isomorphism-based: merged nodes are isomorphic to themselves, so an
isomorphism check structurally cannot see this class of bug.
…thing The parser emits during descent: property lists and collections push triples before the terminating `.` proves the statement well-formed. A statement that failed therefore left its partial triples in the sink, so riot's rule — a rejected statement contributes nothing — held only in the error return, not in the data. `--continue-on-error` cannot be built on that. `abort_statement()` is the counterpart to `end_statement()`, landing now so `--continue-on-error` does not force a second breaking trait change later. Default no-op. The parser calls it only when a statement fails having already emitted; exactly one of the two ends any statement. `GraphCollectorSink` implements it by rewinding the graph to a mark taken at the statement boundary and retiring the statement's literal slots, and `Graph::truncate` is the primitive for that. Writing the test found a real bug in the first cut of this. The parser keeps one token of lookahead, so consuming a statement's `.` lexes the FIRST token of the next statement. A lexical error there was reported while the finished statement was still in flight, and the rollback then discarded a statement that was complete and valid — `ex:a ex:p "1" ; ex:q "2" .` followed by a garbage line lost every triple, not just the garbage. The fix is to commit at the terminator, before the lookahead advance: `end_statement_at_dot` replaces the bare `expect(Dot)` in `parse_triples` and in the dotted directives, and `parse_statement` reports whether it already committed so the caller does not commit twice. A statement that commits and then fails on the advance still reaches `abort_statement`, but the rollback is a no-op because the commit moved the mark past it. Both behaviors are pinned and both tests are mutation-verified: moving the commit back after the advance fails `failure_before_any_emit_leaves_earlier_statements_intact`.
…t_quad The two capability-gated events now behave identically: `debug_assert` in development, `SinkError::Rejected` in release. A dropped reification is data loss in exactly the way a dropped graph name is, so neither default should be a silent no-op. The asymmetry was defensible when written — every producer of reified-triple events checks `supports_reified_triples` and rejects the input up-front, so the default body is unreachable on any path that exists today — but "unreachable" is a property of today's producers, not of the protocol, and it costs nothing to make the backstop real. Covered by a sink that claims the capability without overriding the method, which is what reaches the default past its debug assert.
Two mutations, two different outcomes, both now written down where the next person will look. Recycling IRI slots fails the hostile-document cases, so those are load-bearing. Always reusing literal slot 0 escapes this differential entirely and is caught only by the fluree-graph-ir unit tests — because outside the annotation path the parser emits each literal immediately after minting it, so at most one literal id is live at a time and slot-0-collapse is observationally equivalent here. Worth stating plainly rather than leaving as a passing test: this differential's power is the aliasing dimension, not statement width. If a future producer holds several literal ids at once that stops being true, and this file needs a case for it.
`literal_slots` may only contain slots minted by `add_literal_term`. If a session-scoped term ever lands in one, retiring it hands a producer-cached id to the next literal — and the result is a graph that still looks perfectly well-formed with one term substituted for another, which is the hardest possible failure to notice downstream. So check it where the retirement happens: every slot the statement is about to retire must still hold a `Term::Literal`. Debug builds fail loudly and name the offending slot and term; release builds compile the whole thing out. This is a strictly stronger guard than the differential. Routing `term_iri` through the literal lane trips the assert in `one_sink_many_parses_survives_recycling`, which the output differential alone does NOT catch — the corruption is invisible there but the invariant violation is not. Applied at `abort_statement` too, which retires the same slots.
Everywhere else the parser emits a literal immediately after minting it, so at most one literal id is live at a time — which is exactly why a sink can recycle literal slots per statement without any live id being clobbered. `parse_annotation_tail` is the one exception: `object` may be a literal id and it stays live across the annotation body, which mints further terms including literals of its own, before being handed to `emit_reified_triple`. Unreachable today, because star constructs require `supports_reified_triples()` and the only recycling sink returns false — so no sink both recycles and accepts these events. It goes live the moment a writer sink supports both, which is squarely on this effort's roadmap. Recorded at the site rather than in a design doc, with the two ways out (keep literal ids valid for the whole statement, which `end_statement` already promises; or materialize `object` before parsing the body), because whoever writes that sink will be reading this function, not the doc.
The disjointness test asserted over a document containing both a collection
and a `[ … ]`, which reads like coverage but is not: the spine assertion
fires first, so a regression confined to the bracket path could hide behind
it. The two sites also fail independently — `[ … ]` mints in EVERY mode,
while spine nodes only exist under `CollectionStyle::Spine` — so a
spine-only test would pass with the bracket path broken.
Now each site is asserted on its own document with an explicit distinct-node
count: Spine `( ex:a ex:b )` against `_:b1`/`_:b2` must leave 4 identities,
and default-mode `[ ex:q "anon" ]` against `_:b1` must leave 2. Both were
checked to fail on their own against the old `b{N}` mint — the bracket case
collapses 2 nodes to `["b1"]`.
Counting identities rather than checking isomorphism is the point: merged
nodes are isomorphic to themselves, so an isomorphism check cannot see this
class of bug at all.
The trait publishes "exactly one of end_statement/abort_statement per statement". The parser broke it: the error arm gated only on whether the statement had emitted, while the fact that it had already committed rode back on `parse_statement`'s return value — which the error path discards. A statement that committed at its `.` and then hit a lexical error in the one-token lookahead therefore logged End followed by Abort. Benign today (the commit had already moved GraphCollectorSink's mark, so the rollback was a no-op) and not benign for the buffering sink the hook exists for, which would see a commit revoked. Fixed by moving the answer onto `self` as `committed_current`, which is where it has to live because the case that needs it is the error path. That also removes the redundant `Result<bool>` plumbing through `parse_statement` and both directive parsers — one source of truth, consulted by both arms. Mutation-verified: dropping the `!self.committed_current` guard reproduces the `[End, Emit, Emit, End, Abort]` sequence exactly, and only the new lookahead test catches it.
"Exactly one of end_statement/abort_statement per statement" is a claim about the event sequence, and the previous tests only checked the resulting graph — which is why a spurious Abort survived them: it was a no-op for a collector sink and would only corrupt a buffering one. A `LoggingSink` now records the order and the tests assert it. The old comment was also wrong about its own fixture. It claimed `@@@` fails "at its subject before a single emit"; `@@@` is a LEXICAL error raised while advancing past the PREVIOUS statement's terminator, so it never reaches subject parsing at all. That one document was standing in for two different guards. Split accordingly, one guard each: - `failure_before_any_emit_does_not_abort` uses `"strsubject"` as a subject, which lexes fine and is rejected by `parse_subject` — the only way to fail inside a statement with zero emissions, which is the emit-count guard's true branch. - `lexical_error_in_lookahead_does_not_abort_the_committed_statement` keeps `@@@`, now correctly described, covering the committed-statement guard. - `failed_statement_contributes_nothing` gains the full expected sequence. Also sharpens the recorded invariant behind the slot-0 mutation gap: it is "no literal is minted while another literal id is live", not "one term id at a time". `parse_reified_triple` does hold a possibly-literal object while minting the reifier, but the reifier grammar is `iri | BlankNode`, so no literal is minted there.
The flag's specific failure mode is latching. It is reset at each statement start; if that reset were ever lost it would stay true after the first committed statement and `abort_statement` would never fire again — rollback dying silently, which is strictly worse than the contract bug the flag fixes, because nothing would look wrong. Two guards, both a committed statement followed by one that emits and then fails, so the trailing Abort is mandatory: - a wrong token where the `.` belongs (`ex:q` lexes fine, so the triple is emitted and the statement fails at its terminator); - a missing final dot at EOF, the shape a truncated file produces. Both assert ["End","Emit","End","Emit","Abort"] and one surviving triple. Mutation-verified: removing the reset drops the trailing Abort from both, and from `failed_statement_contributes_nothing`, which is why that test was left alone — it doubles as a latch guard. Also records a case that looks like the wrong-terminator one and is not: `ex:b ex:p "2" @@@` fails BEFORE emitting, because a literal's `@lang`/`^^type` suffix is checked by lexing the next token, so the lexer dies inside the suffix lookahead. No emit, so no Abort — logged as ["End","Emit","End"]. Worth pinning precisely because the two differ only in the failing token.
|
CI note: the |
bplatz
left a comment
There was a problem hiding this comment.
Approving as-is, no changes requested — the protocol design is sound and the adversarial recycling suite is unusually good.
One thing to settle before M1, not in this PR: I think the tool should be fluree convert, not fluree rdf.
fluree exportalready writes Turtle / N-Triples / N-Quads / TriG / JSON-LD, so "RDF" is not what distinguishes the new tool. The real axis is that it is file → file with no ledger in the loop — whichrdfdoes not say, and whichexportwill keep shadowing in a user's head.- Our data-flow commands are verbs (
export,load,insert,query,validate); the nouns are reserved for management groups with sub-verbs under them (graph,branch,remote). A format converter is a data-flow operation, so a top-levelrdfnoun sits against that grain. - CSV already reaches this same
GraphSinkpath viacsv_import.rs→ JSON-LD, and we import Cypher dumps too. A reader for either lands next to this machinery sooner or later, andfluree rdf convert --from csvargues with itself. convertcosts nothing if the scope stays RDF-only and needs no rename if it does not. A--helpline pointing at riot / rapper / rdfpipe keeps the legibility either way.
Nothing here blocks this PR — it ships no CLI surface, which is exactly why now is the cheapest moment to settle the name.
|
Thanks for this — and agreed that now, pre-merge, is the right moment to settle it. Two of your points land cleanly: The complexity that I think changes the calculus: Which is also why I'd read your own grammar observation as supporting the group form: the nouns in our CLI are groups-with-sub-verbs ( One correction to the cost model, in fairness to both sides: the CLI surface does already exist in the stacked PRs above this one (#1555/#1569/#1578 ship The trade-offs as I see them:
Our lean: keep |
PR-0: GraphSink protocol + Turtle ParserOptions
Foundation PR for the
fluree rdf(riot-analog) effort. It lands nothinguser-visible: it changes the shared parser→sink abstraction so the writers,
converters and diagnostics of the later PRs are expressible at all, and it
makes the Turtle parser able to produce conformant RDF as well as the ingest
shape. Blast radius is every ingest path, so the gate is the full transact +
fluree-db-apisuites, not the light crates alone.Implements the locked decisions H-1 (PR-0 split), H-2 (ParserOptions on the
shared parser, ingest pinned to current behavior), and §1.2 of
riot-analog-pr-plan.md(fallible emission,finish(), quad capabilityprobe, term lifetime).
A. GraphSink protocol
Fallible emission.
emit_triple/emit_list_item/emit_reified_triplenow return
Result<(), SinkError>, and the newemit_quaddoes too.term_*stay infallible — they intern a term and hand back an id; a writer only
touches its output on emit.
The point is early termination. Today a sink cannot report that its downstream
died, so
convert big.ttl | head -5would parse to EOF against a closed pipe.SinkErrorcarries anio::Erroror a rejection message and answersis_broken_pipe(), so a CLI can exit quietly on the expected shutdown andloudly on a real failure. The Turtle parser stops at the first refusal and
preserves the sink's error as
TurtleError::Sinkrather than flattening itinto a generic parse error; the JSON-LD adapter does the same via
AdapterError::Sink.Quad capability.
supports_quads()+emit_quad(), mirroring the existingsupports_reified_triplesprobe. No producer emits quads in this PR — this isthe protocol landing, so M2's TriG/N-Quads readers have something to refuse
against.
The default
emit_quadbody errors rather than no-ops. The asymmetry withemit_reified_triple(whose default stays a no-op) is deliberate: everyproducer of reified-triple events already refuses the input up-front on the
capability probe, so that default body is unreachable, not a fallback. Nothing
guards quads yet, so the default has to be the guard. Silently folding named
graphs into the default graph is the exact defect the JSON-LD adapter has
today; this is what stops it being repeated.
Statement lifecycle.
end_statement(), called by the Turtle parser at eachstatement terminator and only for statements that parsed and emitted cleanly.
It serves two later features that need the same boundary: term lifetime (below)
and the statement-scoped output buffering
--continue-on-errorrequires, sincetriples are emitted during descent, before the terminating
.proves thestatement valid. A failed statement never reaches the call, so it contributes
nothing.
finish()and the naming split. The trait gainsfinish(&mut self) -> Result<(), SinkError>— flush and finalize, called bywhoever owns the sink, never by a producer (one sink can be fed by many
producer calls, a chunk each on the bulk-import path).
Three sinks already had an inherent
finish()that consumed the sink andreturned its product. Leaving those named
finish()would have type-checkedwhile letting a concrete-typed caller silently skip the protocol finish —
survivable for a graph collector, not for a writer. They are renamed to
consuming
into_*methods, sofinish()always means "flush" and never "giveme your contents":
GraphCollectorSinkfinish() -> Graphinto_graph()FlakeSinkfinish() -> Result<Vec<Flake>, _>into_flakes()ImportSinkfinish() -> Result<(writer, prefixes, spool), _>into_parts()Deferred-error semantics are unchanged:
FlakeSink/ImportSinkemit_*stillrecord the first invariant violation and keep going, and the consuming method
is still where it becomes a hard error.
Term lifetime.
TermIds now have a documented split. IRI and blank ids aresession-scoped (producers cache them across statements); literal ids are
statement-scoped, because literals are minted per occurrence and never
deduplicated — which is why the term table grew by one live entry per literal
occurrence for the length of the document.
GraphCollectorSinkacts on it:end_statementretires the statement'sliteral slots for reuse, so the high-water mark is the widest single statement
rather than the whole document (1000 statements × 2 literals leaves 4 slots,
not 2002). Soundness rests on two facts, both pinned by tests:
emit_*clonesinto the graph before returning, so overwriting a slot cannot reach an
already-collected triple; and the parser caches only IRI and prefixed-name ids
across statements.
This is groundwork, not a shipped win — no production path benefits today.
FlakeSinkandImportSink, the sinks on the ingest hot path, ignoreend_statemententirely and are deliberately untouched.GraphCollectorSink'susers are the W3C/SHACL testsuite harnesses, the R2RML loader, and
parse_to_json— none of them memory-bound. Producers that never delimitstatements, such as the JSON-LD adapter, never recycle at all and keep today's
allocation behavior exactly. The reason to land it here is that the memory
profile of a writer sink is the differentiator the effort is being built on,
and the protocol hook plus a proven-sound reference implementation is what a
writer needs to exist. Applying it to the ingest sinks is a separate,
separately-benchmarked change.
Statement rollback.
abort_statement()is the counterpart toend_statement(), and lands now so--continue-on-errordoes not force asecond breaking trait change later. The parser emits during descent — property
lists and collections push triples before the terminating
.proves thestatement well-formed — so a rejected statement used to leave partial triples
in the sink; riot's "a rejected statement contributes nothing" held only in the
error return, not in the data. The parser calls it only when a statement fails
having already emitted, and exactly one of
end_statement/abort_statementends any statement.
GraphCollectorSinkrewinds the graph to a mark taken atthe statement boundary.
Writing that test found a real bug in the first cut: the parser keeps one token
of lookahead, so consuming a statement's
.lexes the first token of thenext statement, and a lexical error there was reported while the finished
statement was still in flight — the rollback then discarded a statement that
was complete and valid. Committing at the terminator, before the lookahead
advance, fixes it; the regression is pinned and mutation-verified.
Blank-node namespace.
GraphCollectorSinkminted anonymous blanks asb{N}, the exact shape a document writes by hand, so a file containing_:b1plus any anonymous node produced one node where RDF says two — silently merging
user data into a parser-minted node. Pre-existing via
[ … ]and reifiers, butCollectionStyle::Spinemakes it systematic, since every collection mintsspine nodes. Fixed the way
FlakeSink/ImportSinkalready do it: mint-b{N},which cannot lex as a
BLANK_NODE_LABELand so is unreachable from any input.The tests count distinct identities rather than checking isomorphism, because
merged nodes are isomorphic to themselves and an isomorphism check
structurally cannot see this class of bug.
B. Turtle ParserOptions
The parser canonicalizes in three places that are right for ingest and wrong
for conversion:
triples W3C defines for
( ex:o )arrive as one event;()emits nothing — a silently lost triple, since()denotesrdf:nil;Integer/Doubletokens are parsed to native values, so+1,01,1e0,1.0E0are unrepresentable (Decimaland i64-overflowing integers alreadykeep their lexical form).
Meanwhile subject-position collections have always emitted a proper spine, so
the two positions disagreed with each other.
ParserOptions { collections, numerics }gets both shapes out of one grammar —one conformance surface, no fork.
ParserOptions::default()is today'sbehavior in every field, so every existing caller is unchanged;
ParserOptions::conformant()is the faithful-RDF preset. New entry pointsparse_with_optionsandparse_with_prefixes_base_options(the latter is thefull one — every other
parse*delegates to it with something defaulted, sothe chunked reader paths can opt in too).
CollectionStyle::Spineneeded no new collection code: it drops theobject-list special cases, so
( … )and()fall through to the ordinaryobject path, which already builds a spine and already resolves
()tordf:nil. Both positions now share one code path.One consequence worth naming for review: a spine collection has a single object
term (the head), so the RDF 1.2 annotation-on-collection deferral — which
exists only because indexed items leave nothing to reify — does not apply in
this mode. The deferral is unchanged in the default mode, and both halves are
pinned by a test.
NumericStyle::PreserveLexicalroutes numeric tokens through the typed-stringlane
DecimalandIntegerOverflowalready use. Deviation from the brief,flagged for review: the brief anticipated a new
LiteralValue/Datatypevariant carrying the lexical string. That turned out to be unnecessary — the
existing typed-string lane already represents "this datatype, this exact
lexical", and is the pattern the brief also said to extend. So
fluree-graph-irgains nothing for this feature, the ingest path hits theidentical branch it hits today, and there is no second numeric representation
to keep consistent. Downstream conversion is unaffected:
convert_string_literalalready parses"42"^^xsd:integerback to a nativevalue (pinned by the pre-existing
test_typed_string_integer).Testing
New tests, beyond the existing suites staying green:
there (emits exactly N), the sink's error survives as
TurtleError::Sinkwith
is_broken_pipe()intact, and the failed statement is never terminated.end_statementfires once per completed statement,directives included; a multi-triple predicate-object list is one statement;
the parser never calls the protocol
finish(), and two parses can feed onesink.
supports_quads() == false, andthe default
emit_quadbody returns an error instead of dropping the graphname.
blank ids are not, an already-emitted triple is unaffected by the reuse, and
the term table tracks the widest statement rather than the document; plus an
end-to-end Turtle guard that a later statement reusing an earlier slot cannot
corrupt the earlier values.
rdf:first/rdf:restchain rather than counting triples; one-item,two-item, nested and nested-empty collections;
()in both positions;subject and object positions agreeing; annotations admitted in spine mode and
still refused in the default.
canonicalization still the default; decimals and big integers identical under
both modes; non-numeric literals untouched by the knob.
touch, asserted triple-for-triple in the default mode. This is the regression
guard for "existing callers are unchanged". The pre-existing
test_collection,test_empty_collectionandtest_integer_literalassertions are unmodified and still pass.
The recycling differential
Slot recycling is the one change here whose failure mode is aliasing: a wrong
answer is a perfectly well-formed graph with one term silently substituted for
another. Per-triple expectations written by the author of the recycling are
unlikely to anticipate that, so
fluree-graph-turtle/tests/adversarial_recycling.rsasserts against an oracle instead — the same document parsed into a recycling
sink and into one whose
end_statementis swallowed by a forwarding wrapper,i.e. today's behavior. Any
TermIdoutliving its statement while stillreferenced shows up as a divergence.
Two things make that evidence rather than ceremony:
parser caches across statements — is detected by 2 of the 9 tests in the
file through output comparison alone, and by 9 of 9 once the retirement
assert is present; the 7 additional detections come from the assert
tripping on the polluted slot list, not from any graph diverging. Both
figures were measured by applying the mutation with the assert enabled and
disabled.
entirely and is caught only by the
fluree-graph-irunit tests. The reasonis worth knowing: no literal is ever minted while another literal id is live
(the parser emits each literal immediately after minting it), so
slot-0-collapse is observationally equivalent here. The differential's power
comes from the aliasing dimension, not from statement width. Both facts are
recorded in the module docs so nobody trims the corpus believing the cases
are redundant.
rdf-testsTurtle corpus —1148 files, 718 parsed, 430 rejected, across all three option modes
(default, conformant, spine-only) — zero output divergence from the
no-recycle oracle and zero acceptance divergence, with two temporary
probes live in
end_statement: a poison canary overwriting every retiredslot with a sentinel, and an assertion that every retiring slot still holds a
Literal. Neither probe ships; both are review instrumentation.The corpus harness itself is not in this PR: it hardcodes an absolute path
into the
rdf-testssubmodule. Its home is thetestsuite-rdfharness of theplan's §2, which is where this differential should be re-landed so the result
keeps being enforced rather than being a one-off measurement.
Gates
cargo nextest run -p fluree-graph-ir -p fluree-graph-turtle -p fluree-graph-json-ld -p fluree-graph-format -p fluree-db-transact— 730 passed, 0 failed.cargo nextest run -p fluree-db-api(full, groupedgrp_*bins, no per-filefilters) — 2919 passed, 0 failed, 11 skipped.
cargo clippy --all-targets --no-depson every touched crate — clean(workspace deny-lints, incl.
uninlined_format_args).cargo fmt --all --check— clean.cargo check --all-targetson the excludedtestsuite-sparqlandtestsuite-shaclworkspaces — clean. They call the renamedGraphCollectorSink::finishand would otherwise break in their own CI job,which the workspace gates do not cover.
Review follow-ups deferred to M1
Raised in review, agreed as non-blocking, recorded here so they are not lost:
on
committed_currentare caught by disjoint sets of tests: dropping theguard (aborting a statement that already committed) is caught only by
lexical_error_in_lookahead_does_not_abort_the_committed_statement, whiledropping the per-statement reset is caught only by
failed_statement_contributes_nothingand the twoabort_still_fires_after_a_committed_statement_*cases. Neither sidesubsumes the other, and every one of them looks redundant beside its
neighbours until you actually run the mutations. The test file currently
explains the latch hazard but not this mapping; it wants one comment naming
which test covers which mutation, so nobody trims either side.
after-a-committed-statement rollback cases are already shipped, with
different fixtures — the reviewer reaches the error-at-dot path with a
trailing
,where this PR uses a wrong token in the terminator position.What is not covered here is their
exactly_one_terminator_per_statement_everywheresweep, which asserts theinvariant across bracket, collection, bare-bnode, subject-collection,
SPARQL-prefix and nested-list shapes in a single pass. That sweep is the
valuable remainder.
is "no literal is minted while another literal id is live"; the comment at
parse_annotation_tailstill leads with the looser "at most one literal idis live at a time", which is not literally true (
parse_reified_tripleholds a literal object while minting a reifier — the reifier grammar just
guarantees that term is never a literal). The sharp form is already in the
test module docs; the parser comment should lead with it too.
Not in this PR
No quad producer, no writer sink, no CLI surface, no
--continue-on-error—those are the protocol's first consumers, in M1/M2.
FlakeSink/ImportSinkdonot recycle literal slots and do not implement
abort_statement; both arehot-path changes that want their own benchmarking. The JSON-LD adapter has no
statement boundaries, so it neither recycles nor rolls back, and it still drops
@graphcontents — its module docs now say so accurately (they previouslyclaimed "flattened into the default graph", which is not what the code does),
and fixing it needs the quad protocol this PR lands.