Conversation
`retract_update` issued `DELETE { ?s ?p ?o }` over every node stamped with
the document plus the document node itself. The subject selection is right;
the free predicate was not. Anything else living on those nodes — a trust
weight on the source, a reviewer's note on a relation — was destroyed on the
next run, with the run reporting success and exiting 0.
Scope the sweep to what the emitters actually write: the `doc:`, `nif:` and
`po:` namespaces, plus `rdf:type`, the `rdf:subject`/`predicate`/`object`
reification triple and `rdfs:label`. Anything outside that is somebody
else's and survives.
Scoped by namespace rather than term by term for two reasons. The structure
graph comes from `fluree-doc-model`, a pinned git dependency, so a term list
would go stale on a rev bump and leak the new term silently. And it is the
cheaper shape: warm, on a 46k-triple document, the namespace filter costs
~300ms against ~260ms unfiltered, where enumerating the same 65 predicates
costs 8ms against 3ms on a small ledger and 15-20ms under `VALUES ?p`.
`every_emitted_predicate_is_owned` runs the real parser, chunker and
resolver over a document exercising headings, lists, tables and links and
asserts the contract holds for every predicate they emit, so growing a new
namespace is a test failure rather than a silent leak.
The integration test asserts the end-to-end property rather than the
generated SPARQL: the unit tests here still pass against the old unbounded
sweep, and only the re-ingest test catches it.
Residue, deliberately not fixed here: `rdf:type` and `rdfs:label` on a
pipeline node are swept, because the emitters write them. And the sweep
still under-reaches on `f:reifies*` attachments, which only matters once
annotations are emitted.
`parse_extraction` is deliberately tolerant — one bad item should not cost a chunk — but it was silent in three directions at once, and only one of them is the one that gets reported. - An unknown key on an entity or relation vanished into serde's default. - A relation that failed to deserialize (`"objectIsLiteral": "true"` as a string) was dropped **whole**, with no counter. - Same for an entity, at the sibling site. The second and third are strictly worse than the first and neither had any signal at all. A run against a custom `--system-prompt` asking for a field the schema does not carry printed `0 dropped` and exited 0 — indistinguishable from a clean success. That is what makes `--system-prompt`, `--user-prompt` and `--guidance` promise more customisation than the schema honours. Count both, name the keys — "confidence was ignored" is a fix where "one key was ignored" is a mystery — and report once per run rather than per chunk. Counters ride on `ChunkExtraction` as `#[serde(skip)]`, the treatment `from_cache` already has, so the extraction cache format is unchanged; `resolve` folds them into `ExtractionStats` beside `hallucinated` and `off_model`. Consequence worth knowing: a cache hit reports zero, because the answer was parsed on an earlier run and only the parsed struct is stored. The counts describe this run's parse, and the summary already prints the cache-hit count beside them. Per relation this is one `as_object` on a value `from_value` clones anyway and at most six pointer comparisons — roughly 90 string compares per chunk against a multi-second network call.
`chat_body` hardcoded three fields and each is a way a current model refuses the call: `max_tokens`, which gpt-5 and the o-series reject in favour of `max_completion_tokens`; `temperature: 0`, which the same models reject at any value but their own, so renaming the budget field alone just moves the 400 one field right; and `response_format`, which Anthropic's OpenAI-compatible route 400s on today, against its own published table saying the field is ignored. The fix was already in the file. `responses_body`, eleven lines down, sends none of the three and has never had the problem. Bring `chat_body` in line with it. - `temperature` goes. It was there for determinism, and 0 is already the default on every endpoint that accepts it — so the field bought nothing on those and broke the call on the rest. - `response_format` goes. `json_object` only guarantees syntactic JSON, not a schema, which is exactly what the prompt already asks for and what `clean_json` plus a retry already tolerate. `Request.json` is removed with it rather than left as a field nothing reads. - The output budget stays, under the spelling Chat Completions takes across its whole range. `max_tokens` is the deprecated one. Considered and rejected: declarative `json_mode` / `max_tokens_param` / `temperature` knobs on `ModelEndpoint`. It relocates a correctness problem onto the operator, who would have to re-derive three values every time a url or model changes; it contradicts this crate's own promise that one client covers six target families; and it can only be as right as the capability table someone transcribed it from, which in the Anthropic case is wrong today. `ModelEndpoint` stays a description of where and who, never of what a model can do. What replaces the table is `recover`: a 400 that names a field we sent is the endpoint telling us its dialect, so take the correction and retry once. One rule, for the one field an endpoint can reasonably refuse — a server too old for `max_completion_tokens` gets `max_tokens` back. Deliberately a short list of known refusals and not a general solver: a novel 400 stays an error the operator reads, not a silent mutation of their request. `EXTRACTION_CACHE_VERSION` v2 to v3, which is mandatory rather than tidy. The key covers the prompts, the prompts did not change, and the cache stores the parsed struct — so without the bump every corpus extracted through the malformed body would be served back from disk indefinitely. Also: the summary headline reads `totals.chunks_failed`. A run where every chunk of every document failed printed a green `done:` and exited 0. Chunk failures still do not count toward `totals.failed` — that would exit 1 and break the retry loop — so only the colour and the word change. The tests assert the **serialized body**, matched key set and all, not the struct that produces it. A presence assertion cannot see a field that should not be there, and asserting on the builder's inputs is how this exact bug survives elsewhere.
`ModelEndpoint` derived `Debug` while holding `api_key`, and
`EmbeddingClient` derived it while holding the **resolved** token — `$NAME`
already expanded, so that one prints the live bearer value itself rather
than the config indirection.
Nothing formats either type today; I checked every `{:?}`, `format!` and
`tracing` call in `fluree-db-doc`, `fluree-db-cli` and `fluree-db-mcp`, and
the only one on this path renders a `RelationMode`. So this is a latent
hazard, not a shipped leak. But a derived `Debug` means one future `{:?}`
in a log line, an error message or a panic puts a live key wherever that
goes, and neither type has to be named for it to happen: `DocConfig` holds
three `ModelEndpoint`s and derives `Debug`, so `{config:?}` is enough.
Hand-written impls on both, redacting the secret and keeping everything
useful — including whether a key is configured at all, which is what you
actually want when debugging an auth failure. `LlmClient` already did this
(`llm.rs`); the two types that did not are now consistent with it.
The test renders a literal key and a `$NAME` indirection through the type
itself and through both containers that derive `Debug` over it. That third
case is how `EmbeddingClient` turned up: it is not reachable by grepping
for the field, only by rendering the thing.
Two documents stating `alice knows bob` — one *"because she is his wife"*, one *"reportedly may know"* — come back from extraction byte-identical: `verdict=valid`, `asserted=true`, eleven predicates, none of them evaluative. Corroboration and `doc:verdict` already give a grounded trust filter and already work, but neither can separate the hedged claim from the certain one. That gap is modality, and it is the only one of the six things that get called "confidence" that is both missing and answerable. `doc:assertionMode` ∈ asserted / hedged / attributed / negated, on the review node beside `doc:verdict`, which it is shaped after. An enum rather than a float, deliberately. "How confident are you" is an introspection, badly calibrated, and it collapses three independent axes — did the text say this, how hedged was it, how strong is the relation itself — into one number whose meaning is set by whatever prompt produced it and which cannot be revised afterwards. "Does the source assert, hedge, attribute or negate this" is a classification of text the model is holding. An enum also resists being thresholded as though it were a probability. On the review node, not the edge: two documents stating one triple write one edge, so an edge has no single modality and a merge rule would have to be invented. On the node the question does not arise. Schema-tolerant only — `SYSTEM_PROMPT_TEMPLATE` is untouched. Those strings ship verbatim with Fluree AI's hosted extraction and the provider-side prompt cache is keyed on the exact text, so changing them is not this crate's call. A custom `--system-prompt` can supply the field today, and the prompt edit can land later without reopening any of this. Validated in `resolve` rather than at parse time, so the check still runs on an answer served from the extraction cache. A value outside the four is counted and reported, never stored — an unrecognised mode silently absent is the same failure the previous commit exists to stop. Also declined, and worth being explicit about: a bare `doc:confidence` float. If a caller's prompt emits one it is still reported as an ignored key; we do not ask for it, store it, or invite thresholding on it. No sweep change was needed for the new predicate: re-ingest owns the `doc:` namespace rather than a list of terms in it.
… trust The pipeline has had two grounded trust signals since it shipped — how many independent documents state a triple, and whether the predicate is one the ontology declares — and nothing told anyone they were there, or that they compose into a working traversal filter. Both queries here were run against a real ledger. Source trust is documented as a triple on the document node, joined to the evidence at query time, because that is the only place it can be revised: one triple re-scores every claim a source ever made, where a number baked onto each derived edge at ingest can only be rewritten edge by edge, and is no longer decomposable once written — nothing can tell whether 0.4 meant the source, the sentence or the relationship. Also here: what re-ingest does and does not retract, now that the answer is "what the pipeline wrote, and nothing else" and a curated triple on a pipeline node is a supported thing to have; `doc:assertionMode`, with a prompt fragment that works today and an explicit statement of what it is not; what the request body carries and, more to the point, what it leaves out and why there is nothing to configure about it; and the cache-version note, since an upgrade past this re-extracts every cached corpus once. One thing stated because it cannot be inferred and people will try it: policy cannot express a per-edge trust threshold. Targets bind the subject, so the decision is all-or-nothing per subject and a person with one strong edge and one thin one is either wholly visible or wholly hidden. Trust filtering belongs in the query.
aaj3f
added
bug
`recover` adjusts a refused request and resends it, which is the mechanism that lets one client work against endpoints with different dialects — but it only said so through `tracing::debug!`. A run in which every call was silently downgraded to `max_tokens` read exactly like one in which none was, unless the operator happened to have debug logging on. That is the same shape of silence this PR fixes everywhere else. A counter on `LlmClient`, atomic because one client is shared across the chunk workers, surfaced through `Extractor` and `VlmReader` and reported once at the end beside the extraction counters. Reported outside the extraction block, because crop reading uses the same wire shape: a run with no `--model` at all can still have had its requests adjusted, and should say so. The test drives a server that speaks only the deprecated budget field — 400 naming `max_completion_tokens`, then 200 on the resend — and asserts both the message and the two request bodies, so a message reporting a recovery that did not happen would fail as loudly as a missing one. Also: state the namespace boundary in the docs as a boundary. The retraction paragraph listed `doc:`/`nif:`/`po:` as "what the pipeline itself wrote" and then promised that anything in your own vocabulary survives, which reads as pipeline-terms-only. It is not: the scope is the predicate, not the author, so a `doc:reviewedBy` you added yourself is retracted too. Curate under your own namespace.
bplatz
left a comment
bplatz
left a comment
There was a problem hiding this comment.
Approving — the headline fix is solid, and the test behind it is the strongest thing in the PR.
On the re-ingest data loss (#1868): the scoping is correct. I checked the parts that could have gone wrong:
- The
FILTERsits in the outer group, so it constrains both UNION branches rather than just the second. doc:sourceDocumentis itself insideDOC_NS, so the stamp is swept — otherwise re-ingest would accumulate stale stamps.- No namespace prefix collisions: all ten constants end in
#or/and none is a prefix of another, including thehttps://ns.flur.ee/doc#vshttps://ns.flur.ee/db#pair that could plausibly have bitten.
every_emitted_predicate_is_owned earns its place — it runs the real parser, chunker, resolver and transaction builder rather than asserting over a fixture, and the 12-predicate non-vacuity pin means a fixture that quietly stopped emitting cannot pass while proving nothing. It also proves something the body does not claim: since seen is populated only from document-or-stamped nodes and the pin requires nif:isString and po:contains, structure elements must be stamped, so the sweep demonstrably reaches them.
The Debug redaction is a real latent-hazard fix and correctly scoped — EmbeddingClient held the resolved token, so {config:?} on a DocConfig was enough to leak a live key without either type being named. Finding it by rendering the containers rather than grepping for the field is the right method.
Please look at the two inline notes on llm.rs before merging. Both are on the transport commit and they are really one point:
- (
chat_body) The stated reason for droppingtemperature— "0 is already the default on every endpoint that accepts it" — does not appear to hold for OpenAI Chat Completions, where the default is 1. If so, this makes cold extraction non-deterministic, which sits badly with the content-keyed cache anddoc:extractionFingerprint. - (
recover) The PR builds exactly the mechanism that would fix both 400s without degrading other providers, argues well for why that mechanism beats a capability table, and then removestemperatureandresponse_formatunconditionally instead of routing them through it.
Neither blocks the data-loss fix, which is the reason to land this. Both are worth settling before the transport commit ships, since (1) changes output that users keep.
| // below has always sent. Every field beyond it is a field some | ||
| // current model refuses: | ||
| // | ||
| // - `temperature: 0` was here for determinism, and 0 is already the |
There was a problem hiding this comment.
This justification is load-bearing, and I think the premise is wrong.
temperature: 0was here for determinism, and 0 is already the default on every endpoint that accepts it
For OpenAI Chat Completions the default is 1, not 0. Worth confirming against the live docs rather than either of our recollections — but if it holds, the consequence is not cosmetic.
This pipeline's premise is reproducible extraction. There is a content-keyed extraction cache, a doc:extractionFingerprint on every document node, and an EXTRACTION_CACHE_VERSION that exists to invalidate results when the transport changes. Dropping temperature does not remove a redundant knob in that setting — it makes cold extraction non-deterministic, so two runs over the same corpus on a fresh cache produce different graphs, and the fingerprint stops meaning what it says.
The 400 being fixed is real. The scope of the fix is the issue: gpt-5 and the o-series refuse the field, so drop it for endpoints that refuse it rather than for every provider. That is exactly what recover is for — see the note there.
There was a problem hiding this comment.
Confirmed, and fixed in 0ba8bae69.
You are right and the premise was mine, written from recollection and never checked: the OpenAI Chat Completions default is 1, not 0. So the removal was a regression, not a simplification, and the cache-version bump in this PR guarantees every upgrading user gets exactly the cold run where it shows.
One calibration, because it cuts slightly against you and I would rather state it than let the concession overshoot: temperature: 0 was never a reproducibility guarantee. There is no seed here and no provider promises identical output, so this is near-greedy to fully sampled, not reproducible to not. That does not rescue my claim — near-greedy and fully sampled are far apart on a 15-relation extraction, and your point about the fingerprint claiming more than it can deliver stands exactly as written.
temperature: 0 is sent again and withdrawn only by an endpoint that names it in a 400 — your scope correction, implemented via the mechanism in your other note. Pinned by chat_body_sends_every_field_until_the_endpoint_refuses_one, which matches the whole body by key set so neither an extra field nor a silently-dropped one can hide, and by each_adjustable_field_is_withdrawn_when_the_endpoint_names_it.
The docs now carry the caveat rather than leaving it implied: temperature 0 is near-greedy, not reproducible, and on an endpoint that refuses it you lose even that.
| /// Deliberately a short list of known refusals rather than a general | ||
| /// solver: a novel 400 from a novel model stays an error the operator | ||
| /// sees, not a silent mutation of their request. | ||
| fn recover(body: &mut serde_json::Value, error: &str) -> Option<&'static str> { |
There was a problem hiding this comment.
recover is the right idea, and the PR then doesn't use it for the two fields it actually removed.
The mechanism is well built: one narrow rule, guarded by !recovered so it fires at most once, mutating the body that the retry reuses. No loop risk.
And the reasoning behind it is the part of this PR I most agree with:
a 400 naming a field we sent is the endpoint describing its own dialect, so take the correction and retry once […] it is the only mechanism that survives a provider's published capability table being wrong
That argument applies verbatim to temperature and response_format. Both were removed unconditionally instead — which is a capability judgement hardcoded for every provider, the thing the body argues against two paragraphs earlier. Two more rules here would fix the Anthropic-compat 400s without degrading endpoints that accept the fields:
response_format→ drop on a 400 naming it.json_objectgenuinely improves reliability where it is supported;clean_jsonplus the retry is a weaker guarantee, not an equivalent one.temperature→ drop on a 400 naming it, which keeps determinism everywhere it is available (see the note onchat_body).
This shape is also correct whether or not the Anthropic claim in the body holds today. I could not verify that claim: the reference I have covers the native Messages API, not the OpenAI-compatibility route, and a compatibility surface can change under you — which is the argument for letting the endpoint say so rather than encoding it.
Minor, while you are here: the recovery path falls through to sleep(1 << attempt) at the top of the loop, so a dialect correction waits ~2s it has no reason to, and it spends one of the ATTEMPTS budget that would otherwise cover a genuine transient failure. A dialect fix is not a backoff case.
There was a problem hiding this comment.
Confirmed on all three points, fixed in 0ba8bae69 and b6ffbbb10.
You are right that this is the inconsistency rather than a missing feature: the PR built the mechanism, argued it beats a capability table, then hardcoded a capability judgement for every provider two paragraphs later. Both fields now route through recover, and the budget keeps its rename rule.
One thing your version would have needed, which I do not think the two rules alone give you. recovered is a local in complete (llm.rs:101), so the correction was per call — and complete runs once per chunk. Adding the two rules as written would have cost a wasted round trip, the 2s backoff from your minor note, and one of three retries on every chunk of every document: on a 10k-chunk corpus roughly 10k wasted calls and over an hour of pure sleep at the default concurrency. So the refusal is now remembered on the client for the rest of the run — one extra round trip per field per run instead of per chunk. a_refusal_is_remembered_so_the_next_call_does_not_repeat_it pins it; removing the memo turns it red.
On the matcher, because one of the three rules is weaker than the others and should say so. It prefers error.param, where OpenAI names the field, and falls back to a substring. I have the response_format and max_tokens refusal strings on record verbatim from the reports these rules exist for. I do not have the gpt-5 temperature refusal verbatim — that rule is inferred from the documented behaviour, and both the code comment and the test say so. An inferred matcher marked inferred seems fine; presented as observed it would not be.
Minor finding: also right, and it had a second half. A correction no longer backs off and no longer spends a retry — the second matters more than the sleep, because a correction was making a genuinely transient failure more likely to be fatal by consuming a third of the budget. doc_ingest_keeps_its_full_retry_budget_after_a_dialect_correction pins it: one refusal, two 503s, then an answer, which fails under the old semantics when the budget runs out on the second 503.
Separating the two counters also lets one call settle more than one refusal, which your scenario needs — an endpoint rejecting both temperature and response_format previously had its second refusal turn terminal.
On the Anthropic claim you could not verify: you were right to flag it, and I have not verified it either. Nobody here has made a live call against the compatibility route. The only first-hand evidence is the 400 transcript in #1858. That is now the argument for this shape rather than a premise it rests on — which is your point, and it is why the body no longer asserts provider acceptance as measured.
…uses it An earlier commit here removed `temperature`, `response_format` and the `max_tokens` spelling outright, on the reasoning that `responses_body` sends none of them and works. That reasoning had a premise I did not check and which is wrong: I wrote that temperature 0 "is already the default on every endpoint that accepts it". The OpenAI chat default is 1. That makes the removal a regression rather than a simplification. This pipeline is built on repeatability — the extraction cache is content-keyed, every document node carries a `doc:extractionFingerprint`, and `EXTRACTION_CACHE_VERSION` exists to invalidate answers when the transport changes. Dropping temperature for every provider means a cold run over one corpus yields a different graph each time while the fingerprint says nothing changed, and the cache-version bump in this PR guarantees every upgrading user gets exactly that cold run. It is also inconsistent with this PR's own argument. `recover` was built here, and the body argues that a 400 naming a field is better evidence than a capability table — then two paragraphs later removes two fields by capability judgement, for every provider at once. So: send all three, and withdraw one when the endpoint names it in a 400. `temperature: 0` and `response_format` are back, the budget keeps the current spelling and falls back to `max_tokens` on refusal, and `recover` grows a rule per field. The refusal is remembered on the client, and that is the part that decides the design rather than decorating it. `complete` runs once per chunk, so a per-call correction would cost a wasted round trip, a backoff and one of three retries on every chunk of every document — on a 10k-chunk corpus roughly 10k wasted calls and over an hour of pure sleep at the default concurrency. Remembering makes it once per field per run. Matching prefers `error.param`, where OpenAI names the field, and falls back to a substring. Two of the three refusal strings are on record verbatim from the reports these rules exist for; the gpt-5 temperature one is not, and the code says so — an inferred matcher marked inferred is fine, one presented as observed is not. Determinism deserves its caveat, in the docs and here: `temperature: 0` is near-greedy, not reproducible. There is no `seed` and no provider promises identical output. The regression is still real — near-greedy to fully sampled is a large, visible change — but it was never a guarantee.
`recover` shared the retry loop with 429s and 5xxs, so a correction fell through to `sleep(1 << attempt)` at the top and spent one of the three attempts. Neither is right: an endpoint saying "I do not take that field" is not a condition that improves if you wait, and the retry budget exists to ride out a genuinely transient failure, which a correction has now made *more* likely to be fatal by consuming a third of it. The loop now counts retries and corrections separately. A correction resends immediately and spends nothing; a 429 or 5xx backs off and spends an attempt, exactly as before. That also lets one call settle more than one refusal, which matters for an endpoint that rejects two of the three fields: previously the second refusal was terminal and the chunk failed. Correction terminates without the cap: every rule removes or renames the field it matched, and `recover` only matches a field still in the body. The `ADJUSTABLE.len()` bound guards a future rule that could undo another. Two tests, because the two properties need different evidence. Three refusals settled in one call pins the multi-correction path, and asserts on the gaps *between requests* rather than whole-process wall time — an earlier draft measured the latter and flaked on a cold debug binary, which is the wrong thing to measure for "did this back off". The retry budget needs a transient after a correction to be observable at all, so it gets its own test: one refusal, two 503s, then an answer. Under the old semantics the budget runs out on the second 503 and the chunk is reported failed. Worth recording: my first mutation of this was wrong and passed. Adding `spent += 1` inside the correction branch changes nothing, because the `continue` skips the budget check — so the test comment claiming four requests were "arithmetically impossible" under the old behaviour was false, and is gone. The faithful mutation puts the check and the sleep back on the correction path, and both tests catch that.
|
Self-filed correction, not raised in review. A sweep of our own found a claim in this PR body that a reader could not check, and it is the kind that most needs to be checkable. Correcting it turned up a second problem underneath, so both are here. One: a stale test name. The verification section said:
That test had zero occurrences at the head it was written against. It was a real pre-existing test on Two: the mutation itself had gone stale. Settling this by re-running rather than by guessing which name I meant is what exposed it. The mutation was "reintroduce the three refused fields" — meaningful only against the design in an earlier revision of this PR. At the head that shipped, those fields are sent deliberately and withdrawn on a 400, so that mutation no longer says anything and a reader could not reproduce it either. Correcting a citation onto a design that had moved underneath it would have been the same defect in a new place. The figures now in the body are from the equivalent mutation against the design that shipped — removing
That second one is the pre-existing test, and its blindness is the whole point of naming it — it is why an absence assertion had to be added rather than assumed. A third, in the opposite direction — a mutation that passed and should not have. Reproducing the old "a correction spends a retry" behaviour by adding |
2 participants
fluree doc ingesthad four problems that compound: a re-ingest destroyed data it did not own, a failed extraction reported success, the request body could not talk to the model this crate's own config example names, and a custom--system-promptasking for a field the schema does not carry got silence back.Fixes #1858
Partially addresses #1864
Partially addresses #1868
Follow-up: #1860
Six commits, one per concern, each self-contained.
Re-ingest deleted triples it did not write — #1868
The most serious thing here.
graph::retract_updateissuedDELETE { ?s ?p ?o }over every node stamped with the document plus the document node itself. The subject selection was right. The free predicate was not: anything else living on those nodes went with it.Verified live before the fix —
ex:trust 0.9on a document node andex:reviewedConfidence 1.0on a relation node both committed, then vanished after re-ingesting an edited source, with the run printingdone: 1 ingested, 0 unchanged, 0 failedand exiting 0. No warning on stdout or stderr.The sweep is now bounded to what the emitters actually write: the
doc:,nif:andpo:namespaces, plusrdf:type, therdf:subject/rdf:predicate/rdf:objectreification triple, andrdfs:label. Anything outside that is yours and survives.Scoped by namespace rather than term by term, for two reasons. The structure graph is emitted by
fluree-doc-model, a rev-pinned git dependency that writes nineteendoc:terms this repo never names — six of them PDF-only, so unreachable from any markdown fixture. A term list would go stale on a rev bump with nothing able to see it. And it is the cheaper shape: warm, eight filter terms beat sixty-five.every_emitted_predicate_is_ownedruns the real parser, chunker and resolver over a document exercising headings, lists, tables and links, assembles the actual transaction, and asserts the contract holds for every predicate they produce. A new namespace is a test failure naming the offender. It also asserts the fixture reached twelve specific predicates, one per emitter, so a fixture that quietly stopped emitting anything cannot pass while proving nothing.The test that matters asserts the end-to-end property — curate, edit the source, re-ingest, query — rather than the generated SPARQL. That distinction is load-bearing, and it is sharper than "the unit tests are weaker": the unit tests on
retract_updatepin the generated text, and only the behavioural test pins what that text does. A mutation that rewrites the filter fails them both, because the namespace strings disappear from the update. A mutation that leaves the text intact and neuters its meaning — making the filter a tautology — passes both unit tests and is caught only by the re-ingest test, which goes from 3 rows to 0. The second is the mutation worth defending against, because it is the shape a real regression takes.On #1868, and why not
Fixes#1868 is one construct with two opposite failures, and this PR closes one of them.
Closed: the over-reach. The wildcard no longer deletes triples the pipeline did not write. That is the data-loss half, and it is the half the design depends on — putting source trust on the document node and composing it at query time does not work at all while the wildcard stands.
Not closed: the under-reach. A variable predicate provably cannot surface
f:reifies*facts, so re-ingest still cannot retract an annotation attachment, and a corroborated edge can still be left with a body-less reifier. Scoping?pdoes not change that — the predicate is hidden from the scan either way. It is inert today because nothing in this pipeline emits annotations, and it becomes live the moment--promotedoes, which is why it belongs with that work rather than this.A note for anyone reading the issue first: it proposes the fix as a source-marker three-way diff, after solo's
evidence.rs. I did not take that shape, and the reason is worth stating. A node-level marker cannot close the over-reach, because the loss is at triple granularity on nodes the pipeline itself mints —urn:fluree:doc:memo-a.mdwould carry the marker, andex:trust 0.9on it is exactly the triple that has to survive.doc:sourceDocumentis already a source marker on every emitted node; the subject half of the old construct was never the defect. The free predicate was. The marker shape becomes the right one for the under-reach, where the unit really is a node the pipeline owns outright — so it is the annotation work's to adopt, not this PR's to pre-empt.Two pieces of residue beyond that, stated rather than hidden.
rdf:typeandrdfs:labelon a pipeline node are still swept, because the emitters write them; there is no fix short of per-triple provenance.Extraction dropped things silently, in three places
parse_extractionis deliberately tolerant — one bad item should not cost a chunk — but it was silent in three directions, and only one of them is the one #1864 reports:"objectIsLiteral": "true"as a string) was dropped whole, with no counter;The last two are strictly worse than the first and neither had any signal at all. A run against a custom prompt asking for a field the schema does not carry printed
0 droppedand exited 0 — indistinguishable from a clean success. That is what made--system-prompt,--user-promptand--guidancepromise more customisation than the schema honours.All three are now counted, and unknown keys are named — "confidence was ignored" is a fix where "one key was ignored" is a mystery. Reported once per run, not per chunk. Counters are
#[serde(skip)], the treatmentfrom_cachealready has, so the extraction cache format is unchanged; the consequence is that a cache hit reports zero, which the cache-hit count on the same line explains.The request body could not talk to gpt-5, or to Anthropic
Three hardcoded fields in
chat_body, three separate 400s:max_tokens, which gpt-5 and the o-series reject in favour ofmax_completion_tokens;temperature: 0, which the same models reject at any value but their own — so renaming the budget field alone just moves the 400 one field right, which is the correction fluree doc ingest: hardcoded response_format and max_tokens block current models, and the failure reports as success #1858 needs and does not have;response_format: {"type":"json_object"}, whichapi.anthropic.com/v1400s on today, against its own published compatibility table saying the field is ignored.Each is sent, and withdrawn when the endpoint refuses it.
{ "model": "gpt-5-mini", "messages": [ … ], "temperature": 0, "response_format": { "type": "json_object" }, "max_completion_tokens": 8000 }A 400 naming one of those fields is the endpoint describing its own dialect. That field is withdrawn — or, for the budget, renamed to
max_tokens— the request is resent immediately, and the refusal is remembered for the rest of the run. An unrecognised 400 stays a terminal error reported verbatim, not a silent rewrite of the operator's request.Remembering is what makes this affordable rather than a per-chunk tax.
completeruns once per chunk, so a per-call correction would cost a wasted round trip, a backoff and one of three retries on every chunk of every document — on a 10k-chunk corpus roughly 10k wasted calls and over an hour of pure sleep at the default concurrency. Remembering makes it one extra round trip per field per run.A correction also no longer backs off or spends a retry: an endpoint saying "I do not take that field" is not a condition that improves if you wait, and the retry budget exists for transients.
An earlier revision of this PR removed all three unconditionally, and that was wrong. The premise I gave — that temperature 0 "is already the default on every endpoint that accepts it" — is false; the OpenAI chat default is 1. Removing it for every provider would have made a cold run over one corpus produce a different graph each time while
doc:extractionFingerprintclaimed nothing had changed, and the cache-version bump below guarantees every upgrading user gets exactly that cold run. It was also inconsistent with this PR's own argument:recoverexists precisely so a capability judgement does not have to be hardcoded for everyone. Caught in review by @bplatz.Considered and rejected: declarative
json_mode/max_tokens_param/temperatureknobs onModelEndpoint. It relocates a correctness problem onto the operator, who would have to re-derive three values every time a url or model changes; it contradicts this crate's own promise that one client covers OpenAI, Ollama, vLLM, LM Studio, Voyage and the Fluree AI gateway; and it can only be as right as the capability table someone transcribed it from — which in the Anthropic case is wrong today.ModelEndpointstays a description of where and who, never of what a model can do.What replaces a table is
recover: a 400 naming a field we sent is the endpoint describing its own dialect, so take the correction and retry once. One rule, for the one field an endpoint can reasonably refuse — a server too old formax_completion_tokensgetsmax_tokensback. Deliberately a short list of known refusals rather than a general solver: a novel 400 stays an error the operator reads, not a silent mutation of their request.EXTRACTION_CACHE_VERSIONv2 → v3 is mandatory, not incidental. The key covers the prompts, the prompts did not change, and the cache stores the parsed struct rather than the raw answer — so without the bump every corpus extracted through the malformed body would be served back from disk indefinitely, forever, with no way to tell. This is by definition a transport fix andcache.rsalready states that contract.And the headline now reads
chunks_failed: a run where every chunk of every document failed printed a greendone:. Chunk failures still do not count towardtotals.failed— that would exit 1 and break a retry loop that works — so only the colour and the word change.The API key was one
{:?}away from a logModelEndpointderivedDebugwhile holdingapi_key, andEmbeddingClientderived it while holding the resolved token —$NAMEalready expanded, so that one renders the live bearer value rather than the config indirection.Nothing formats either type today. I checked every
{:?},format!andtracingcall influree-db-doc,fluree-db-cliandfluree-db-mcp; the only one on this path renders aRelationMode. So this is a latent hazard, not a shipped leak — but a derivedDebugmeans one future{:?}in a log line, an error message or a panic puts a live key wherever that goes, and neither type has to be named for it to happen:DocConfigholds threeModelEndpoints and derivesDebug, so{config:?}is enough.Hand-written impls on both, redacting the secret and keeping everything useful, including whether a key is configured at all.
LlmClientalready did this; the two types that did not are now consistent with it.EmbeddingClientwas found by the test rendering the containers, not by grepping for the field.Relations now carry how the source states them
Two documents stating
alice knows bob— one "because she is his wife", one "reportedly may know" — come back from extraction byte-identical:verdict=valid,asserted=true, eleven predicates, none of them evaluative. The only thing distinguishing them isdoc:excerpt, unstructured text.Corroboration and
doc:verdictalready give a grounded trust filter that works today — a filtered traversal correctly admits a two-source relation while excluding a one-source one — but neither can separate the hedged claim from the certain one. That gap is modality.doc:assertionMode∈asserted/hedged/attributed/negated, on the review node besidedoc:verdict.An enum, not a float. "How confident are you?" is an introspection, and a badly calibrated one; it also collapses three independent axes — did the text really say this, how hedged was it, how strong is the relation itself — into one number whose meaning is set by whatever prompt produced it. "Does the source assert, hedge, attribute or negate this?" is a classification of text the model is holding. An enum also resists being thresholded as though it were a probability, and matches the precedent already in the vocabulary:
doc:verdictis an enum, not a score.On the review node, not the edge: two documents stating one triple write one edge, so an edge has no single modality and a merge rule would have to be invented. On the node the question does not arise.
Schema-tolerant only —
SYSTEM_PROMPT_TEMPLATEis untouched. Those strings ship verbatim with Fluree AI's hosted extraction and the provider-side prompt cache is keyed on the exact text, so editing them invalidates a cache this repo neither owns nor can measure, and changes what hosted accounts ask of a model without those accounts redeploying. That is not this PR's call to make, and it does not have to be: a custom--system-promptsupplies the field today, the docs carry a working fragment, and the prompt edit can land later without reopening any of this.On #1864, and why not
Fixes#1864 has two halves. The silent-drop half is fully fixed, and larger than filed. The other half asks for a
doc:confidencefloat, and this PR declines it — so closing the issue would misrepresent what shipped.The float collapses three independent axes into one number, it is a model self-report rather than an observation, and it is unrevisable once baked onto a node: a source you later learn to distrust can only be reflected by rewriting every edge it produced. The two grounded signals it would sit beside — corroboration and
doc:verdict— are better evidence and already exist. If a caller's prompt emits aconfidenceanyway, the run now says it was ignored; we do not ask for it, store it, or invite thresholding on it.That is a judgement worth disagreeing with, which is exactly why the issue stays open.
Docs
Corroboration and
doc:verdictas the primary trust signals, with both queries — run against a real ledger, not written from memory. Source trust on the document node, composed at query time, because one triple there re-scores every claim a source ever made where a number baked onto each edge can only be rewritten edge by edge. Whatdoc:assertionModeis and is not. What the request body carries, what it leaves out, and why there is nothing to configure about it. And one thing that cannot be inferred and people will try: policy cannot express a per-edge trust threshold, because targets bind the subject, so a person with one strong edge and one thin one is either wholly visible or wholly hidden. Trust filtering belongs in the query.Upgrading
Two things an existing user should know before running this:
temperature: 0is still sent, and is dropped only by endpoints that refuse it. On gpt-5 and the o-series it is withdrawn automatically after one 400, and extraction there is fully sampled — so a cold run can produce a different graph each time while the fingerprint is unchanged. Everywhere else the near-greedy behaviour is unchanged from before this PR. Worth stating plainly:temperature: 0was never a reproducibility guarantee — there is noseedand no provider promises identical output — but near-greedy and fully sampled are far apart.Not here
--promoteand RDF 1.2 annotation output. #1860 stays open. The retraction machinery it was blocked on is in this PR; the emission half waits on the annotation lane-selection work being tracked separately.Also not addressed:
MAX_TOKENS = 8000is a reasoning and output budget on a reasoning model, so it can be spent before any visible text is produced, and the empty answer lands in the same swallowed-chunk path as a 400. The headline change makes such a run non-green and the per-chunk reason is already printed, so it is diagnosable — but the budget is still not configurable.Verification
Ran locally, green:
cargo test -p fluree-db-doc(68);cargo test -p fluree-db-cli(370 across all five targets, including 143 integration);cargo test -p fluree-db-docs(22);cargo clippy --all-targets --no-depsonfluree-db-doc,fluree-db-cli,fluree-db-mcpandfluree-db-docs, zero findings each;cargo fmt --all --checkafter the last edit. Every commit was also checked out on its own and re-tested.Did not run: workspace-wide
cargo testand workspace-widecargo clippy— both are red on this toolchain for unrelated reasons, so I scoped to the crates touched plus every crate that depends onfluree-db-doc. Nothing intestsuite-sparqlwas touched, so it was not formatted or linted separately.No live LLM call was made, at any point. The available key is invalid and provider spend was not authorised. The wire format is verified two ways instead, both offline: unit tests that match the whole serialized body by key set (not field-by-field — a presence assertion cannot see a field that should not be there, and asserting on the struct that builds the body is precisely how this bug class survives elsewhere), and an integration test through a local capturing
chat/completionsserver that asserts the request as actually transmitted by the built binary. What is proven is what the binary sends. That the corrected body is one gpt-5 and Anthropic accept is inferred from their published contracts and from the 400s in #1858, not measured — nobody has made a live call against either. The claim this PR stands on is the first one, which is checkable and sufficient: the three fields removed are each documented as rejected, and the field kept is documented as accepted across the range. Confirming acceptance needs one manual call each with a working key, and should be recorded here before merge.Mutations, each reverted and watched fail, with needles built from post-
cargo fmtsource. Two are worth naming because something passed.Reintroducing the unbounded sweep: the distinction is sharper than "the unit tests are weaker". The unit tests on
retract_updatepin the generated text; only the behavioural test pins what that text does. A mutation that rewrites the filter fails them both, because the namespace strings vanish from the update. A mutation that leaves the text intact and neuters its meaning — making the filter a tautology — passes both unit tests and is caught only by the re-ingest test going 3 rows to 0. The second is the shape a real regression takes.Removing
temperaturefromchat_body— the regression @bplatz caught — gives 7 passed, 4 failed inllm::tests. The four failures are the tests that assert on which optional fields go out. Of the seven passers, five are unrelated to the chat body at all (two parse response envelopes, three exercise the Responses builder, a different function), and two are blind to the change:an_unrecognised_refusal_changes_nothing, which compares the body before and after and so is agnostic to what it contains, andchat_body_carries_system_and_user_turns, which asserts the system turn, the user turn and the model and nothing about the optional fields. That second one is the pre-existing test frommain, and its blindness is the point — it is why an absence assertion had to be added rather than assumed.Correcting my own citation, twice. An earlier revision of this section named
chat_body_carries_system_and_json_modeas the survivor. That test does not exist at this head: it was a real pre-existing test this PR renamed, so the result was real and the name stale. A reader cannot distinguish a stale name from a claim that was never run, and a mutation that survives is evidence about coverage — which has to be reproducible to be evidence at all. True and unverifiable is worse than a weak check.Settling it by re-running rather than by deciding which name I meant then turned up the second problem: the mutation itself was "reintroduce the three refused fields", which was meaningful only against the design in an earlier revision of this PR. At this head those fields are sent deliberately, so that mutation no longer says anything and a reader could not reproduce it either. The figures above are from the equivalent mutation against the design that actually shipped. Both found by our own sweep, not raised in review.
A third mutation is worth recording because it passed and should not have: adding
spent += 1inside the correction branch, intended to reproduce the old "a correction spends a retry" behaviour, changes nothing — thecontinueskips the budget check. The faithful mutation restores the check and the sleep on that path, and both correction tests catch it. The test comment that claimed four requests were "arithmetically impossible" under the old behaviour was wrong and is gone.Perf. Extraction is unchanged per chunk and per relation — roughly a hundred pointer comparisons against a multi-second network call. The retraction path costs 10–15% more, once per re-ingested document. That is my own measurement and has not been independently reproduced: a 185 KB document with 46,319 triples on stamped nodes, sweep query ~260 ms → ~300 ms warm, whole update 1.463 s → 1.568 s (min of three, debug build, local machine). It does not degrade with size — per-row cost is under a microsecond, which is what eight prefix tests per row should come to. Worth re-running if the number matters to a decision.