# ADR-0026: Extending the ADF Validator with Quantifiers, Attributes, and Marks

## Status

✅ Accepted (2026-05-10)

## Context

[ADR-0023](adr-0023.md) introduced the ADF schema validator: a static
per-parent allowed-children table plus a depth-first walker that flagged
nesting violations. Three classes of structural mistake were explicitly
deferred to a follow-up:

- **Quantifiers / arity.** Empty `bulletList`, two-`media` `mediaSingle`,
  `layoutSection` with one column.
- **Mark whitelists per context.** `code` mark on heading text, `border`
  block mark on a paragraph, malformed `link.href`.
- **Attribute schemas.** `panel.panelType: "purple"`, `heading.level: 7`,
  `taskItem.state: "INPROGRESS"`, missing required `localId` /
  `extensionType` / `id`.

[Issue #733](https://github.com/rust-works/omni-dev/issues/733) tracks
this work. The single open question was *how* to encode each class —
hand-written tables (continuing the ADR-0023 pattern) or a pulled-in
parser/schema library — and *how* to surface new violations through the
existing public API without a thrash of ad-hoc enum variants.

## Decision

### Three sub-PRs against one branch

The work lands as three commits / sub-slices in the order:

1. **Quantifiers** (carries the `AdfSchemaViolation` struct→enum
   refactor that the other two slices need).
2. **Attributes** (introduces shared `AttrSchema` / `AttrType` /
   `AttrProblem` machinery).
3. **Marks** (reuses the attribute machinery for per-mark attrs).

Order rationale: the breaking refactor lives in slice 1, so callers
update once. Slice 2 ships the shared attr-validation primitives that
slice 3 reuses for `link.href` / `subsup.type` / `indentation.level`
checks — doing slice 3 second would mean inventing the same machinery
twice.

### `AdfSchemaViolation` becomes an enum

ADR-0023's single-struct violation type is replaced with a five-variant
enum (`DisallowedChild`, `Arity`, `MissingAttr`, `InvalidAttr`,
`DisallowedMark`, `InvalidMarkAttr`). Reasons:

- Each new check carries different payload (arity needs counts, attrs
  need a `problem`, marks need an `inline_index`). A struct with
  `Option<...>` fields per use case would silently allow nonsense
  combinations.
- Callers can opt in to strictness by matching only on the variants
  they care about — the issue's "callers can choose strictness"
  requirement.
- The first variant `DisallowedChild { child_type, parent_type, path }`
  preserves the existing field shape verbatim, so the migration is
  mechanical: `AdfSchemaViolation { ... }` → `AdfSchemaViolation::DisallowedChild { ... }`.

`Display` dispatches per variant. `path()` is a shared accessor
returning the index path from the document root.

### Quantifiers — hand-written `ContentExpr` enum, no parser

Considered:

- **Pull in a content-expression parser** (e.g. `prosemirror-content`
  ported to Rust). Discarded: ~25 entries hand-curated in
  `adf_schema.rs`, no benefit over data tables, adds a translation
  layer.
- **Plain `&[&str]` per parent (the existing v1 form)** plus a side
  table of "minimum count per parent." Discarded: too coarse — doesn't
  represent `mediaSingle: media (caption)?` (a sequence of two terms)
  or `layoutSection: layoutColumn{2,3}` (a range).
- **Custom `Quantifier` enum + `ContentTerm` records.** Selected.

`Quantifier` is `{ZeroOrOne, ZeroOrMore, OneOrMore, Exactly(n),
Range(min, max)}`. `ContentTerm` is `{ atoms: &'static [&'static str],
quant: Quantifier }`. A parent's content model is `&'static
[ContentTerm]` — a sequence of quantified atom-or-alternation terms.
Walker is greedy NFA-style: each child advances `current_term`
monotonically; per-term counts are tallied; arity violations are
emitted in a second pass.

**Lenient deviations from upstream**, all documented inline:

- `doc` is `block*` (upstream `block+`) so `AdfDocument::new()` —
  the canonical empty value for missing JIRA descriptions — does not
  trigger an arity violation.
- `tableCell` / `tableHeader` are `block*` (upstream `block+`) because
  visibly-empty cells in real Confluence tables are common and accepted
  by the renderer.

**Disallowed children do not satisfy parent arity.** A panel containing
only an `expand` produces both `DisallowedChild` (for the `expand`)
and `Arity` (for the panel needing ≥1 valid child). The user clearly
tried to put something there but it's the wrong type — flagging both
is principled and lets the user fix one without missing the other.

**`unsupportedBlock` / `unsupportedInline` count toward parent arity.**
A panel containing only an `unsupportedBlock` validates cleanly: the
preservation wrapper satisfies the `+` requirement. This preserves
ADR-0020's round-trip guarantee for unknown node types.

### Attributes — hand-written `AttrSchema`, not `schemars::JsonSchema`

`schemars` is already a workspace dependency (used by
`src/claude/response_schema.rs` to *generate* JSON Schema for OpenAI
structured outputs). Considered reusing it as a *consumer*:

- **Derive `JsonSchema` on per-node typed attribute structs.**
  Discarded: `AdfNode.attrs` is intentionally `serde_json::Value` so
  the converter and round-tripper can preserve unknown fields
  (ADR-0020). Introducing typed structs would either fork the type
  (one for the converter, one for the validator) or strip unknown
  fields on the validator side.
- **Bring in a JSON-Schema *validator* crate** (e.g. `jsonschema`).
  Discarded: a new crate tree just to validate ~25 small per-node
  schemas, with no win over a hand-written table.
- **Hand-written `AttrSchema` enum.** Selected.

`AttrSchema { fields: &'static [(&'static str, AttrType,
AttrPresence)] }` lists each declared field with its type and
required/optional flag. `AttrType` covers the shapes the upstream
schema actually uses: `Enum`, `IntRange`, `NumRange`, `Bool`,
`String`, `Url` (parsed via the `url` crate already in deps),
`Object`, and a `Free` escape hatch for attrs whose shape we have
not audited.

`AttrProblem` (`NotInEnum`, `OutOfRange`, `OutOfRangeF`, `WrongType`,
`BadFormat`) centralises Display wording so each new schema doesn't
spawn a new error string.

**Permissive on unknown fields.** Only declared fields are checked.
Atlassian sometimes adds optional fields that omni-dev's snapshot
doesn't yet describe; flagging them would generate noise without
helping callers.

**Explicit `null` is treated as absent.** Matches Atlassian's payload
conventions where `localId: null` and "no `localId`" are
indistinguishable.

### Marks — same `AttrSchema` machinery, allow-lists per context

Mark validation has two layers:

- **Allow-list by context.** Inline marks (those on text nodes etc.)
  are checked against the *parent's* inline-mark allow-list — `code`
  is fine on `paragraph` text but not on `heading` text. Block marks
  (those on paragraph/heading/tableCell etc.) are checked against the
  node's *own* block-mark allow-list — `border` and `backgroundColor`
  are valid on `tableCell` / `tableHeader` only; `alignment` and
  `indentation` are valid on `paragraph` / `heading` only.
- **Per-mark attribute schema.** Each known mark type has its own
  `AttrSchema` (reused from the attribute slice). `link` requires an
  absolute-URL `href`; `subsup` requires `type ∈ {sub, sup}`; `border`
  requires `size ∈ 1..=3` and a hex `color`; etc.

**Strict, faithful-to-the-JSON-schema by default.** The user
explicitly accepted this recommendation when reviewing the plan: the
validator flags marks the JSON schema rejects, even if the renderer
would silently drop them. A lenient mode (`validate_document_with_options`)
can be added later if real-world false-positive feedback warrants it.

**`unsupportedMark` and `unsupportedNodeAttribute` are accepted under
any context.** Same round-trip preservation rationale as
`unsupportedBlock` / `unsupportedInline`.

### Hints extracted to `adf_hints.rs`

The `hint_for(parent, child)` helper that lived in `confluence_api.rs`
since #717 grows three more match arms (one per new violation class)
and risks bloating an unrelated file. Move it to a dedicated
`src/atlassian/adf_hints.rs` module that takes `&AdfSchemaViolation`
and dispatches per variant. Each new sub-PR only touches the variant
it owns, keeping diffs small.

### Cross-cutting decisions

- **`PartialEq` only, not `Eq`.** `AttrProblem` carries `f64` (for
  `NumRange` / `OutOfRangeF`); since `f64` doesn't implement `Eq`,
  neither can `AdfSchemaViolation`. `PartialEq` is sufficient for all
  internal uses (test asserts, vec comparisons).
- **Public `validate_document` signature unchanged.** `Vec<AdfSchemaViolation>`
  is still the return type; the variant extension is purely additive at
  the call site (provided callers stop using struct construction).
- **No CLI surface change.** No new flag is added. The `update-snapshots`
  skill is *not* triggered.
- **Diagnosis still selects the first violation.** The HTTP-500
  diagnosis path in `confluence_write_error` still picks
  `validate_document(...).into_iter().next()`. With more variants in
  play, the chosen "first" may now be a `MissingAttr` rather than the
  `DisallowedChild` it was historically — fine, since the dry-run path
  shows the full list pre-flight, and the diagnosis line names the
  variant explicitly.

### Scope cuts

Out for this round, deferred to follow-ups:

- **Per-violation-class grouping in `--dry-run` output.** Currently
  one flat list. If user feedback says it's noisy, regroup later.
- **Lenient mode for marks.** `validate_document_with_options` with a
  `strict_marks: bool` can be added if real-world false positives
  show up.
- **Mark-attr URL constraints.** `link.href: "javascript:..."` and
  `data:` URLs are accepted today (only "is it parseable as a URL" is
  checked). A separate slice can add scheme allow-listing if needed.
- **Auto-fix.** The validator reports problems; it does not edit the
  document.
- **Selecting a "best" violation for HTTP-500 diagnosis.** Possible
  future work: prefer `DisallowedChild` over `MissingAttr` so the
  user sees the structurally-most-impactful issue first.

## Consequences

**Positive:**

- **The validator now catches the three classes of mistake the issue
  enumerated.** Empty bullet list, two-media mediaSingle, panelType:
  "purple", heading level 7, code-on-heading-text, malformed
  link.href — all flagged with named violation variants.
- **Shared attr-validation machinery.** Per-node and per-mark
  attribute checks share `AttrSchema` / `AttrType` / `AttrProblem`,
  which means future schema additions land as one-line table entries.
- **Forward-compat preserved everywhere.** Unknown node types,
  unknown attribute names, unknown marks, unknown parents — all
  permissive. ADR-0020's round-trip guarantee remains intact.
- **Auditable transcription.** Each `CONTENT_ENTRIES`,
  `ATTR_ENTRIES`, `MARK_ATTR_ENTRIES`, `INLINE_MARKS_ENTRIES`,
  `BLOCK_MARKS_ENTRIES` entry cites its upstream source file. Refresh
  reviews remain line-by-line tractable, the same pattern ADR-0023
  established.

**Negative:**

- **Manual transcription drift continues to be a risk.** As before,
  pinned by `SCHEMA_VERSION` and `UPSTREAM_TARBALL_SHA256`; CI drift
  detection against upstream remains a separate effort
  ([#731](https://github.com/rust-works/omni-dev/issues/731)).
- **Test fixtures across the codebase needed updating.** Many
  pre-existing tests constructed nodes via local helpers that
  produced attr-less panels, headings, etc. — technically invalid
  ADF that v1 tolerated. Fixed in this slice with `with_attrs` /
  `panel` / `media` / `layout_column` test helpers in
  `adf_schema.rs::tests`.
- **A document with one disallowed child can now produce two
  violations** (the `DisallowedChild` + an `Arity` for the parent
  needing ≥1 valid child). Pluralisation in `print_dry_run` already
  copes; existing tests that pinned exact violation counts were
  updated.

**Neutral:**

- **No new crate dependencies.** `url` (used by `AttrType::Url`
  validation) was already a dependency. The `schemars` crate was
  considered and rejected — see "Attributes" decision above.
- **Public-API surface change is one struct→enum migration.** Callers
  outside the crate see exactly that one breaking change. The variant
  set will continue to grow (PR-by-PR) without further breakage.
