Skip to content

feat(schema): type FallowOutput root enum, derive document-root oneOf from Rust (#384)#391

Merged
BartWaardenburg merged 3 commits into
mainfrom
feat/issue-384-typed-fallow-output
May 18, 2026
Merged

feat(schema): type FallowOutput root enum, derive document-root oneOf from Rust (#384)#391
BartWaardenburg merged 3 commits into
mainfrom
feat/issue-384-typed-fallow-output

Conversation

@BartWaardenburg
Copy link
Copy Markdown
Collaborator

Summary

Item 6 of the #384 schema-derive ladder: the document-root oneOf in docs/output-schema.json is now derived from a typed FallowOutput enum in crates/cli/src/output_envelope.rs, replacing the previously hand-maintained 12-entry block.

  • 11 object-shaped envelope variants (AuditOutput, ExplainOutput, ReviewEnvelopeOutput, ReviewReconcileOutput, CoverageSetupOutput, ListBoundariesOutput, HealthOutput, DupesOutput, CheckGroupedOutput, CheckOutput, CombinedOutput) live behind #[serde(untagged)] so the wire stays byte-identical; consumers continue to narrow by unique field presence.
  • CodeClimateOutput (bare-array form per the Code Climate / GitLab Code Quality spec) is excluded from FallowOutput and lives as a sibling root branch.
  • CoverageAnalyzeOutput is hand-maintained pending #384 item 3c and tracked by a new HAND_MAINTAINED_ROOT_ENVELOPES constant plus hand_maintained_root_envelopes_appear_in_root_one_of drift test so it cannot silently disappear from the documented union.
  • The regenerated TypeScript contracts (editors/vscode/src/generated/output-contract.d.ts, npm/fallow/types/output-contract.d.ts) now expose FallowOutput as a discriminated-union typename for downstream codegen consumers.

A future major release plans to add a top-level kind discriminator field for true O(1) narrowing on AI / agent consumers, paired with a one-cycle --legacy-envelope opt-out flag.

Design context

The discriminator question (untagged-now vs tagged-now) went through a 7-persona panel review. Verdict: ship untagged now, plan the tagged switch for the next major with an opt-out cycle. Untagged was the right call because (a) wire-shape change in a minor would burn renovate/CI users for a marginal narrowing win, (b) the maintainability win of retiring the hand-maintained root block is the real headline, (c) the planned future tag flip can land independently. The panel also caught that CodeClimateOutput cannot be a variant of any internally-tagged enum and CoverageAnalyzeOutput is a separate (item 3c) migration that should not be absorbed into this PR.

Variant order inside FallowOutput is most-specific first so a validator narrowing the oneOf does not match a CheckOutput payload against CombinedOutput first (CombinedOutput's required set is a strict subset of CheckOutput's).

Verification

  • cargo test --workspace --all-targets (1471 + 437 + 290 + 154 + ... all pass)
  • cargo clippy --workspace --all-targets -- -D warnings clean
  • cargo clippy -p fallow-cli --features schema-emit --bin fallow-schema-emit --tests -- -D warnings clean
  • cargo test -p fallow-cli --features schema-emit --bin fallow-schema-emit (9/9 drift tests, including 2 new)
  • cargo fmt --all -- --check clean
  • cargo doc --workspace --no-deps --document-private-items clean
  • pnpm run codegen:types + pnpm run check:codegen + pnpm run lint clean in editors/vscode/
  • python3 -c "from jsonschema import Draft7Validator; Draft7Validator.check_schema(json.load(open('docs/output-schema.json')))" clean
  • Real-world wire validation: fallow check --format json against benchmarks/fixtures/real-world/preact validates against the regenerated schema with 0 errors
  • OLD / NEW release binaries are hash-identical (the only Rust change is a #[derive]-only enum that is never constructed at runtime today)

Out-of-scope follow-up

AJV --strict=true flags one pre-existing strictRequired error on ContributorEntry.format (the field is in required but missing from properties). Exists identically on origin/main; not introduced by this PR.

Team review

Reviewer Verdict Notes
rust-reviewer CONCERN Stale doc comment + variant-order risk (both fixed in e3c05d3a)
json-output-reviewer APPROVE Pre-existing ContributorEntry.format AJV issue flagged as out-of-scope
cli-output-reviewer CONCERN 4 prose polish items (all fixed in e3c05d3a)
vscode-reviewer APPROVE TS contracts compile, no breakage in extension consumers

Consensus: 2 APPROVE + 2 CONCERN, 0 BLOCKs. Both CONCERN reviewers' fixes applied in the follow-up commit.

🤖 Generated with Claude Code

@BartWaardenburg
Copy link
Copy Markdown
Collaborator Author

Codex's parallel /fallow-review on 30c0d849 caught a real BLOCKER my own review missed: the discriminator examples in the new prose (docs/output-schema.json root description, FallowOutput doc comment, CHANGELOG, backwards-compatibility) named report for health and groups for dupes. Both are wrong: HealthOutput and DupesOutput flatten their body via #[serde(flatten)], so the actual top-level keys are body fields like health_score (health) and clone_groups (dupes), not wrapper keys.

Fixed in 30c0d849 with correct discriminator field names across all four prose locations, plus a clarifying note that the two flattened envelopes carry the discriminator on body fields. Schema and TS contracts regenerated.

Also filed two follow-ups for pre-existing schema gaps surfaced during my self-review (both exist identically on origin/main, not introduced by this PR):

BartWaardenburg added a commit that referenced this pull request May 18, 2026
… (Refs #384)

Migrates 6 dependency-family dead-code findings (`UnusedDependency`/`UnusedDevDependency`/`UnusedOptionalDependency` over the shared `UnusedDependency` struct; `UnlistedDependency`, `TypeOnlyDependency`, `TestOnlyDependency`) off the post-pass `actions` injection and onto typed `*Finding` envelope wrappers. The three `UnusedDependency` views share `build_unused_dependency_actions` which swaps the primary fix from `remove-dependency` to `move-dependency` when `used_in_workspaces` is non-empty; the four `Add to ignoreDependencies` suppress actions share `build_ignore_dependencies_suppress_action`. `SuppressKind::ConfigIgnoreDep` and the `IGNORE_DEPENDENCIES_VALUE_SCHEMA` constant retire as dead code. Wire shape stays byte-identical. After this PR the augmentation table holds 1 finding type (`UntestedFile` / `UntestedExport` will retire with PR #391's root-enum work) plus 3 dupe entries. Refs [#384](#384).
… from Rust (Closes #384)

Every object-shaped --format json envelope (AuditOutput, CheckOutput,
CheckGroupedOutput, CombinedOutput, DupesOutput, HealthOutput, ExplainOutput,
CoverageSetupOutput, ReviewEnvelopeOutput, ReviewReconcileOutput,
ListBoundariesOutput) is now a variant of a typed `FallowOutput` enum in
crates/cli/src/output_envelope.rs with `#[serde(untagged)]` +
`#[derive(JsonSchema)]`. The schema-emit binary's new
`rewrite_document_root_one_of` function derives the document-root `oneOf` from
this enum, replacing the previously hand-maintained 12-entry block.

Wire-identical: `#[serde(untagged)]` preserves every envelope's existing
top-level shape byte-for-byte. Consumers continue to narrow by unique field
presence (`summary.total_issues` for check, `report` for health, `groups` for
dupes, `check`+`dupes`+`health` keys for the bare combined invocation,
`boundaries` for list --boundaries). Real-world smoke on vite / preact / zod
fixtures + jsonschema validation on the regenerated schema confirm zero
behavioral delta.

CodeClimateOutput stays as a sibling root branch (`#[serde(transparent)]
Vec<CodeClimateIssue>` is required by the Code Climate / GitLab Code Quality
spec and cannot be a variant). CoverageAnalyzeOutput remains hand-maintained
pending #384 item 3c; a new `HAND_MAINTAINED_ROOT_ENVELOPES` constant plus
two new drift tests (`hand_maintained_root_envelopes_appear_in_root_one_of`,
`root_one_of_carries_fallow_output_and_codeclimate`) ensure both stay
reachable from the documented union.

A future major release plans to switch FallowOutput to
`#[serde(tag = "kind")]` for true O(1) discrimination on AI / agent consumers,
paired with a one-cycle `--legacy-envelope` opt-out flag.

Regenerated TS contracts (editors/vscode + npm) expose `FallowOutput` as a
discriminated union for downstream codegen consumers.
Rust-reviewer:
- Reorder FallowOutput variants most-specific first. CombinedOutput is LAST
  because its required-field set (schema_version, version, elapsed_ms) is a
  strict subset of every other variant's required set; placing it earlier
  would let untagged narrowing (and any future Deserialize) match a
  CheckOutput payload against CombinedOutput first. CheckGrouped is now also
  ordered before Check so the grouped_by discriminator narrows in the right
  order.
- Fix stale doc comment: HAND_MAINTAINED_ALLOW_LIST -> HAND_MAINTAINED_ROOT_ENVELOPES.

CLI-output-reviewer:
- CHANGELOG: lead with the user-visible artifact (the schema file), drop the
  nested **Wire-identical** bold to match the surrounding Internal-section
  style.
- CONTRIBUTING.md: add the file path on first mention of
  rewrite_document_root_one_of, replace the "internally tag" serde jargon
  with its plain-English meaning.
- backwards-compatibility.md: drop the serde-attribute leakage
  (#[serde(untagged)] / #[serde(tag = "kind")]) from a consumer-facing doc;
  use "no top-level discriminator field today" and "a top-level kind
  discriminator field" instead. Keep the FallowOutput typename because
  consumers read it in the schema and TS contracts.
- docs/output-schema.json root description: restructure so the narrowing
  procedure leads the paragraph instead of living in a parenthetical aside.
  Drop "untagged" from the consumer-facing description.

JSON-output-reviewer and vscode-reviewer both APPROVED with no changes
requested.

Regenerated docs/output-schema.json + TS contracts to match.
@BartWaardenburg BartWaardenburg force-pushed the feat/issue-384-typed-fallow-output branch from 30c0d84 to 80d0281 Compare May 18, 2026 13:35
#384)

The narrowing examples in docs/output-schema.json's root description, the
FallowOutput doc comment, the CHANGELOG entry, and docs/backwards-compatibility.md
named `report` for health and `groups` for dupes as the unique top-level
discriminators. Both are wrong:

- HealthOutput flattens HealthReport into top-level fields
  (`#[serde(flatten)] report: HealthReport`), so the wire has no `report`
  key; actual top-level keys include `findings`, `health_score`,
  `hotspot_summary`, `large_functions`. The most distinctive choice is
  `health_score`.
- DupesOutput flattens DuplicationReport into top-level fields
  (`#[serde(flatten)] report: DuplicationReport`), so the wire has no
  `report` key; actual top-level keys include `clone_groups`,
  `clone_families`, `stats`, `mirrored_directories`. The most distinctive
  choice is `clone_groups`. The previously-suggested `groups` is only
  present when `--group-by` is set on dupes (rare) and clashes with the
  same key name on health-grouped output.

Replaced with correct field names across all four prose locations, plus
a clarifying note that HealthOutput and DupesOutput flatten their bodies
so the discriminator is a body field (not a wrapper key). Schema and TS
contracts regenerated.

Caught by Codex's parallel /fallow-review pass.
@BartWaardenburg BartWaardenburg force-pushed the feat/issue-384-typed-fallow-output branch from 80d0281 to 2802620 Compare May 18, 2026 13:47
@BartWaardenburg BartWaardenburg merged commit 44b6030 into main May 18, 2026
20 checks passed
@BartWaardenburg BartWaardenburg deleted the feat/issue-384-typed-fallow-output branch May 18, 2026 13:54
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.

1 participant