Skip to content

Content provenance for decoded values - #582

Merged
gordonwoodhull merged 3 commits into
mainfrom
feature/yaml-provenance
Aug 24, 2026
Merged

Content provenance for decoded values#582
gordonwoodhull merged 3 commits into
mainfrom
feature/yaml-provenance

Conversation

@gordonwoodhull

@gordonwoodhull gordonwoodhull commented Aug 23, 2026

Copy link
Copy Markdown
Member

Problem

SourceInfo ranges recorded for decoded values were computed as if the decoded text were the source text. That holds only while a value's content matches its source byte for byte, which decoding routinely breaks:

  • backslash escapes collapse two source bytes to one content byte ("a\*b"a*b)
  • YAML block scalars strip a newline plus the block indent per line
  • quoted attribute values have delimiters that aren't part of the value

Two independent bugs followed from the same bad assumption, both in resolveChain on the TS side:

  1. Concat exclusive end. The end offset was derived by mapping the last content character and adding one. With a trailing two-byte escape the +1 lands mid-escape: {tail="y\*"} resolved to [49,51] (y\) instead of [49,52] (y\*). A value whose escape isn't last resolves correctly either way, which is why no existing test caught it.
  2. Substring-over-Concat. A Substring's local offsets were composed onto its parent's resolved start — affine composition over the hull. Inside a YAML block scalar every inline after a line break drifts by the bytes the newline-plus-indent collapse removed: the second line's Str "line" resolved to [24,28] ( li) instead of [26,30]. This reaches consumers through inline-converter.ts's per-inline getAnnotatedParseSourceFields.

Affine composition over a hull is the shared defect. It had already been found and fixed once, then reintroduced at three more sites.

Approach

Give decoded content its own provenance rather than deriving it arithmetically from the node.

  • ConfigValueKind::Scalar becomes a struct variant carrying content_source_info: Option<SourceInfo> alongside yaml. 241 call sites swept.
  • AttrSourceInfo.attributes[i].1 becomes a SourceInfo over the decoded value instead of a Range over the node. The attribute path is driven from the markdown decoder via ProvenanceBuilder.
  • resolveChain's two arms are replaced by mapContentRange(id, start, end), which walks a half-open content range down to source coordinates: Original shifts into file space, Substring shifts into the parent's content space, and Concat unions the contributions of the pieces it overlaps. A piece whose declared content length equals its own extent contributed verbatim and can be indexed into; any other piece is a replacement, is opaque, and contributes its whole source span.
  • span_assert resolves Concat and Substring{parent: Concat} piecewise, and stops reporting OutOfBounds when the start offset is past EOF.

Requires quarto-source-map 0.1.3 and quarto-yaml 0.1.3 for four map_offset fixes; quarto-error-reporting 0.2.2.

Audit

The third commit enumerates every offset→location conversion and range composition — by consumer, since a producer's range can only be judged against what reads it — and fixes what it found:

  • comrak Text-node drift. comrak merges entity and escape runs into one Text node, so a position-derived offset drifts by the collapsed bytes for every node after the first in a run. Replaced with a walker that advances source and content positions in lockstep.
  • empty_source_infoGenerated, not Original(0, 0..0). An absent location was indistinguishable from byte 0 of the first file, so diagnostics anchored at unrelated text.
  • offset_to_location_bytes floored — the third and last offset→location rounding rule.
  • codeblock_shorthand body search bounded to between the fences, so it can't match text belonging to an enclosing container.
  • is_gapless narrowed to the queried sub-range rather than answering for the whole span.
  • Dead shortcode_string range deleted; q_2_28/q_2_33 moved to the resolving accessor.

Breaking

@quarto/annotated-qmd 0.1.1 → 0.2.0. An attribute value's range no longer includes its quotes — the range now describes decoded content, not the source node. Consumers trimming a character off each end should stop.

Review notes

  • 20 annotated-qmd example fixtures regenerated. Uniform cause: meta scalars now emit a separate content-provenance pool entry, so the pool grows and later s indices shift by the count inserted ahead of them. Verified structurally — Original (type-0) entry counts unchanged in every file; only type-1 entries added. main's full-path name field preserved.
  • 4 pampa JSON snapshots move the same way; 002/003 also drop a stale insta assertion_line.
  • One expectation from main changed. test_div_kv_keys_have_tight_source asserted "Values keep their quotes: P1 includes a node's own delimiters." That is exactly what this PR changes. The key assertions in that test — main's tight-attribute-key fix (bd-1d6io) — are untouched and pass. content-provenance.test.ts pins the new quote-trimmed ranges case by case, including both failure modes above.
  • 12 tuple-form ConfigValueKind::Scalar(...) sites added on main during the rebase window were swept. They merge cleanly and then fail to compile, so they're invisible to a conflict count.

Verification

cargo nextest run --workspace: 13130 passed, 199 skipped, 0 failed, against a live main baseline of 13045/198/0. The +85 is 86 added − 1 removed. The removal is callout::tests::escaped_title_span_stays_inside_the_attribute, which pinned the attribute_value_source length-arithmetic workaround this PR deletes.

cargo xtask verify: all steps passed — custom lints + clippy, rustfmt, Rust build, tree-sitter tests, CRLF parity, ts-packages build, WASM build, hub-client build + tests, preview-renderer integration (51/51 files, 591 passed), trace-viewer, q2-preview-spa.

The wasm32 test target is checked separately, since cargo check --all-targets on a native host never compiles it:

cargo build -p pampa --test wasm_lua --target wasm32-unknown-unknown \
  --no-default-features --features lua-filter -Zbuild-std=std,panic_unwind,panic_abort

This caught a real defect in the first commit's sweep, now fixed: wasm_lua.rs matched on ConfigValueKind::scalar(..), the constructor function, in a pattern position (error[E0164]). The blanket Scalar(scalar( rewrite is correct for construction but invalid in a match arm, and the file is #![cfg(target_arch = "wasm32")], so no native build sees it. Patterns now use the struct variant. Repo-wide scan confirms this was the only pattern-position occurrence.

Design notes and the audit classification: claude-notes/plans/2026-08-20-provenance-{1,2,3}-*.md, claude-notes/research/2026-08-21-provenance-audit-findings.md.

@posit-snyk-bot

posit-snyk-bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

A source range recorded against a YAML scalar was only correct while the
decoded value matched its source bytes one for one. It often doesn't: an
escape collapses two bytes to one, and a block scalar drops a newline
plus its indent on every line. Anything mapping a decoded offset back to
the file then landed short, and the error grew with each collapse.

ConfigValueKind::Scalar becomes a struct variant carrying
`content_source_info` next to `yaml`, so the decoded content carries its
own provenance instead of being assumed byte-identical to the source.
241 call sites updated for the new shape.

span_assert now resolves Concat and Substring{parent: Concat} spans
piecewise rather than composing them affinely over the enclosing hull,
which is what produced wrong spans for these values, and no longer
reports OutOfBounds when the start offset is past EOF.

Requires quarto-source-map 0.1.3 and quarto-yaml 0.1.3 for four
map_offset fixes, and quarto-error-reporting 0.2.2. A tripwire test pins
the Concat exclusive-end value across the bump.

Design notes for this and the two following commits are in
claude-notes/plans/2026-08-20-provenance-{1,2,3}-*.md.
…mxa44voa)

Carries content provenance out to the code that reads it, so positions
reported to users and tools point at the right bytes.

- Attribute values: an attribute's recorded range no longer includes its
  surrounding quotes, and survives escapes in the value. Driven from the
  markdown attribute decoder via ProvenanceBuilder.
- Link and image titles get their own bound spans.
- fig-cap caption content gets provenance through the cell-options path.
- Both YAML re-parse paths thread content provenance; quarto-config's
  now-unused YAML converter is made private so the two cannot diverge.
- Config sources bind by root_file_id rather than resolve_byte_range.
- A YAML string scalar arriving with no content provenance now warns.
  It is non-fatal and un-coded: it signals an internal inconsistency,
  not something a user can act on, and the render still completes.
- A panic while rendering one diagnostic no longer takes down the run.
- Provenance is excluded from the incremental-rebuild hash, so recording
  it does not defeat caching.

TypeScript reader: `resolveChain` composed a Substring's local offsets
onto its parent's *resolved* start, and derived a Concat's exclusive end
by mapping the last content character and adding one. Both hold only
while content matches source byte for byte. Concretely, `{tail="y\*"}`
resolved to `y\` instead of `y\*` — the +1 landed inside a trailing
two-byte escape — and every inline after a line break in a YAML block
scalar drifted by the bytes the indent collapse removed. Replaced with
`mapContentRange`, which walks a half-open content range down to source
coordinates. @quarto/annotated-qmd 0.1.1 -> 0.2.0, breaking.

SNAPSHOT CHANGES: 4 modified, 0 added, 0 removed.
  crates/pampa/snapshots/json/{002,003}.snap,
  horizontal-rules-vs-metadata.snap, table-caption-attr.snap

  All four move the same way: meta scalars now emit a separate content
  provenance pool entry, so the pool gains entries and every later `s`
  index shifts by the number inserted ahead of it. No range value
  changes meaning. 002/003 also drop a stale insta `assertion_line`
  header. The 20 ts-packages/annotated-qmd/examples/*.json fixtures are
  regenerated for the same reason.
…(bd-mxa44voa)

Audits every site converting an offset to a location or composing
ranges, and fixes the ones that were wrong. Findings and the 26-row
classification are in
claude-notes/research/2026-08-21-provenance-audit-findings.md.

- comrak merges entity and escape runs into a single Text node, so a
  position-derived offset drifted by the collapsed bytes for every node
  after the first in that run. A lockstep walker now advances source and
  content positions together.
- `empty_source_info` becomes Generated rather than Original(0, 0..0),
  so a missing location stops masquerading as byte 0 of the first file
  and pointing diagnostics at unrelated text.
- `offset_to_location_bytes` is floored, closing the third and last
  offset-to-location rounding rule.
- codeblock_shorthand's body search is bounded to between the fences, so
  it cannot match text belonging to a surrounding container.
- `is_gapless` is narrowed to the sub-range actually queried instead of
  answering for the whole span.
- The dead shortcode_string range is removed; q_2_28 and q_2_33 use the
  resolving accessor.
- The per-diagnostic guard in render.rs is wrapped.

Covered end to end by a regression test for the original crash, plus
selective-replay coverage.
@gordonwoodhull
gordonwoodhull force-pushed the feature/yaml-provenance branch 2 times, most recently from f5591bb to 6f4a230 Compare August 23, 2026 22:40
@gordonwoodhull
gordonwoodhull merged commit 596ceb5 into main Aug 24, 2026
17 of 18 checks passed
@gordonwoodhull
gordonwoodhull deleted the feature/yaml-provenance branch August 24, 2026 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants