Accept TriG on insert (API, server, CLI) - #1971

🟡 OPT-1 — the not-TriG path copies the whole document twice to conclude "not TriG"

(the parse_trig_phase1 call in stage_trig_insert)

The bail-out logic here is right — a plain-Turtle typo should come back as a Turtle error, and the named_graphs.is_empty() && raw_meta.is_none() guard makes sure it does. What I'd like to tighten is what it costs to reach that conclusion.

parse_trig_phase1 short-circuits on might_contain_graph_block, but the short-circuit still allocates (trig_meta.rs:200-206):

if !might_contain_graph_block(input) {
  return Ok(TrigPhase1Result {
      turtle: input.to_string(),   // full copy of the document
      raw_meta: None,
      named_graphs: Vec::new(),
  });
}

…and might_contain_graph_block itself allocates a second copy whenever the document has no {, which is exactly the plain-Turtle case (trig_meta.rs:441-448):

if input.contains('{') { return true; }
input.to_ascii_uppercase().contains("GRAPH")   // full uppercase copy

So every Turtle insert that fails to parse now clones the body twice and throws both away. Your code didn't introduce either allocation — they're pre-existing in trig_meta.rs and upsert has been paying them all along — but this PR is what routes the insert error path through them, and insert is the lane where a large body is most likely. A 500 MB Turtle file with a syntax error near the end goes from "parse, fail, return" to "parse, fail, uppercase-copy 500 MB, copy 500 MB, return".

Cheapest fix is to not enter the helper at all when it can't help. might_contain_graph_block is private (no callers outside trig_meta.rs, I checked), so either expose it and gate the call:

      if !fluree_db_transact::might_contain_graph_block(trig) {
          return Err(turtle_err);
      }
      let phase1 = fluree_db_transact::parse_trig_phase1(trig)?;

…or, probably better since it fixes upsert too, make the helper allocation-free and let TrigPhase1Result::turtle be a Cow<'_, str>:

fn might_contain_graph_block(input: &str) -> bool {
  if input.contains('{') {
      return true;
  }
  input
      .as_bytes()
      .windows(5)
      .any(|w| w.eq_ignore_ascii_case(b"GRAPH"))
}

I recognize this is non-blocking — but if you agree it's right, I'd rather see it folded in now than lost in the backlog.

The two allocations live in fluree-db-transact/src/parse/trig_meta.rs, which isn't in this diff; commenting on the call site that newly reaches them.

Folded in (c255cfe), with both halves of what you suggested.

might_contain_graph_block now scans bytes in place: {, or a case-insensitive GRAPH over a five-byte window. It is exported, and stage_trig_insert returns the Turtle error before calling parse_trig_phase1 when it says no. A plain-Turtle typo now comes back without either copy. The in-place scan also drops the uppercase copy from every plain-Turtle upsert.

I left the Cow in TrigPhase1Result alone. It would remove upsert's remaining pass-through copy, but it touches upsert's plumbing, which felt outside this PR. A unit test pins the scan matching the keyword in any case and ignoring plain Turtle.

Correcting myself above: the Cow turned out to be small, so it's in too (6694f81). TrigPhase1Result::turtle is now a Cow<'_, str> that borrows the input when the document cannot contain a graph block. It is owned only when phase 1 rebuilt the Turtle. None of the three callers changed, since the insert fallback, upsert_turtle and TriG import all read it as &str. With that, a plain-Turtle upsert no longer copies its body at all, and phase1_borrows_a_document_with_no_graph_blocks pins the borrow.

🟡 OPT-2 — the sync / validate TriG refusal only fires on an extension or a flag

(the sniff_data_format(content) tail of detect_data_format)

trig_refused() is a genuinely nice error and I like that it names the commands that can place blocks. The gap is that it's only reachable through the two explicit routes: --format trig, or a .trig extension. Content sniffing still returns Turtle for anything non-JSON, and the new comment above it says so deliberately:

Content sniffing. A TriG body sniffs as Turtle, which is fine: the Turtle entry points read graph blocks too.

That reasoning is exactly right for insert and upsert — both now match Turtle | Trig to the same arm, so misdetection is harmless there. It doesn't hold for the three commands that refuse TriG. cat data.trig | fluree sync <ledger> --graph <g> (or any path without a .trig suffix) sniffs as Turtle, goes to fluree_graph_turtle::parse_to_json in SyncSource, and dies with failed to parse Turtle: … at the first GRAPH — which is the same opaque dead end your own detect.rs comment calls out for N-Quads:

An N-Quads file would otherwise sniff as Turtle and die in the parser on its fourth term, which tells the reader nothing.

The reasoning applies here verbatim, so I think the sniffer should distinguish the case it's now being asked to distinguish:

fn sniff_data_format(content: &str) -> CliResult<DataFormat> {
  if serde_json::from_str::<serde_json::Value>(content).is_ok() {
      Ok(DataFormat::JsonLd)
  } else if might_contain_graph_block(content) {
      Ok(DataFormat::Trig)
  } else {
      Ok(DataFormat::Turtle)
  }
}

…which costs nothing for insert/upsert (same arm) and gets sync, validate and --shacl the good message. If reaching for the transact-crate helper is more coupling than you want in detect.rs, a local content.contains('{') is probably enough for a sniffer — it only has to be better than "always Turtle".

This is more of a question than a suggestion if you'd decided piping TriG into sync isn't worth the branch. But given the effort in the rest of this diff to replace opaque parser errors with commands-you-should-use-instead errors, it felt like the one place it doesn't land.

Agreed, and it was worse than an opaque error for validate. Its Turtle arm calls insert_turtle, which reads TriG as of this PR, so a TriG body under a .ttl name was silently loaded into named graphs rather than refused.

Fixed in c255cfe, but not by sniffing. might_contain_graph_block is a "might": a { inside a string literal, such as JSON stored as a value, would make valid Turtle sniff as TriG, and then sync would refuse it. Instead the three one-graph commands check the body with detect::refuse_trig_body, which runs parse_trig_phase1 and refuses only when it finds graph blocks or txn-meta. insert and upsert are untouched, since TriG and Turtle share their arm.

• validate and --shacl check before loading.
• sync checks only once the Turtle parse has failed, which TriG always does, so its success path is unchanged.

a_trig_body_under_another_name_is_refused_by_one_graph_commands covers:

• TriG piped to sync;
• a TriG .ttl file given to validate, and to --shacl;
• a Turtle file with {v} in a literal, which still validates.

With the validate check removed, the test fails on validate exiting 0.

🔵 INFO-1 — two behavior improvements the consolidation quietly buys

Not a finding — a note for the PR body, because I had to diff the old and new bodies to convince myself these were intentional rather than accidental, and the next reader will too.

Pointing OwnedTransactBuilder::execute() at stage_core changes the InsertTurtle lane in two ways beyond removing the duplicate dispatch. The old code returned early into insert_turtle_with_opts before a tracker was constructed and before the store_raw_txn handling, so:

• tracking options set on an insert_turtle builder were silently ignored; they now take effect (tracker_ref is threaded through),
• store_raw_txn was a no-op on that lane; it now spawns the upload via maybe_spawn_txn_upload.

Both look like fixes to me rather than regressions, and the staging semantics are otherwise identical — I compared the new stage_core arm against the old insert_turtle_with_opts body and the stage-then-commit_stage_result sequence matches. Worth a line in the description since "about 190 fewer lines" undersells it.

Deliberate, and you're right that it deserved a line. The body now says so under the stage_core section: tracking options on an insert_turtle builder, and store_raw_txn on that lane, both now take effect where they were silently ignored.

⚪ NIT-1 — ACCEPTED_FORMATS is the right move, and one spelling is missing from it

Hoisting the accepted-format list to one const so the two usage errors can't drift is good, and the_usage_error_names_every_format_the_flag_accepts is a nice guard for exactly the failure mode the comment above nquads_help describes.

Small thing: the extension branch accepts .nt (routed to Turtle) but ACCEPTED_FORMATS — which is what both usage errors print — doesn't mention nt anywhere. Someone with a .nt file who reaches for --format nt gets "unknown data format 'nt'" listing formats that don't include the one their file is. Either add nt to the --format match beside turtle | ttl, or name it in the const. Genuinely minor.

Fixed in c255cfe. --format nt is accepted next to turtle | ttl, and ACCEPTED_FORMATS lists it. the_usage_error_names_every_format_the_flag_accepts includes nt, and the --format rows in the insert, upsert and sync docs mention it.