Add post-filter FBC invariant validators by joelanford · Pull Request #30 · release-engineering/fbc-update-planner

joelanford · 2026-07-01T20:49:43Z

Summary

Add four post-filter invariant validators that reject FBC packages not meeting consumer contracts: packages must have versions, versions must have phases, phases must have non-nil dates, and phases must be contiguous (no gaps or overlaps)
Add Equal and NextDay methods to the Date type for contiguity checking
Update reference test data (34 → 20 valid packages; rejections are mostly same-day phase boundaries that need upstream PLCC fixes)

This PR is stacked on top of #29 (the structured FBC types commit). The relevant commit to review is the second one.

Test plan

All existing tests pass (make test)
Table-driven tests for each validator (accept + reject paths)
Contiguity tests cover one-day gap, one-day overlap, single phase, and multi-version cases
Reference file integration tests updated to reflect stricter output

Replace plain string fields in the FBC output types with structured types that enforce format invariants at construction time: - Version.Name: string → MajorMinor (uint64 Major/Minor, regex-validated) - Phase.StartDate/EndDate: string → *Date (wraps time.Time, YYYY-MM-DD) - Platform.Versions: []string → []MajorMinor Translation functions (translatePhase, translateVersion, newPackage) now return errors instead of silently passing through malformed data. Errors are collected with errors.Join (no fail-fast) and surfaced as validation failures. Empty timestamps and "N/A" translate to nil *Date (not an error). This enforces the FBC lifecycle contract — the schema that downstream consumers depend on — which is separate from the PLCC validation policy that governs data quality in the upstream source. PLCC validators check whether product lifecycle data meets content policy (tier requirements, date contiguity, release cadence, etc.); the FBC type layer ensures the output schema is well-formed by construction regardless of which PLCC validators are enabled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Reject packages where: - no versions exist - any version has no phases - any phase has nil start or end date - phases within a version are not contiguous (endDate+1 != next startDate) These run after mutation filters and cannot be disabled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

fullsend-ai-review · 2026-07-01T20:53:06Z

🤖 Finished Review · ✅ Success · Started 8:53 PM UTC · Completed 9:06 PM UTC
Commit: feb9bda · View workflow run →

fullsend-ai-review · 2026-07-01T21:06:32Z

Review

Findings

High

[stale-doc] docs/VALIDATION_RULES.md:25 — VALIDATION_RULES.md states: "Filters mutate packages to produce clean FBC output. They do not perform validation — data quality checks belong in the PLCC validators." This PR adds 4 validator functions (ValidatePackageHasVersions, ValidateVersionsHavePhases, ValidatePhaseDates, ValidatePhaseContiguity) to DefaultFilters(). The default pipeline table (line 31–33) only lists FilterIncompletePhases. The "Adding a New Filter" section (line 99–101) also states filters are "for output cleanup only." All three statements become factually incorrect after this PR, actively misleading developers who consult this document before modifying the pipeline.
Remediation: Update VALIDATION_RULES.md to (1) revise the "They do not perform validation" statement, (2) add the 4 new validators to the default pipeline table, (3) update the "Adding a New Filter" section.

Medium

[stale-doc] CLAUDE.md — Multiple statements are now incorrect: (a) Gotchas section states "newPackage() silently converts unparseable timestamps to empty strings" — it now returns errors. (b) Architecture section states the FBC filter pipeline has "No validation logic — that belongs in PLCC validators" and "Currently only FilterIncompletePhases" — it now has 5 filters including 4 validators. (c) Layout section describes filter.go as "Output cleanup pipeline — mutation-only filters" — no longer accurate.
Remediation: Update CLAUDE.md to reflect that newPackage now returns errors for invalid data and the FBC filter pipeline includes both mutation filters and invariant validators.
[breaking-api] pkg/fbc/fbc.go:42 — Exported struct field types changed: Version.Name (string → MajorMinor), Phase.StartDate/EndDate (string → *Date), Platform.Versions ([]string → []MajorMinor). These are breaking changes for Go code importing pkg/fbc. JSON wire format is preserved (MajorMinor marshals as string, Date marshals as YYYY-MM-DD, nil *Date marshals as null). This is a pre-v1 module so breaking changes are permitted but should be documented.
Remediation: Document the Go API breaking changes in the PR description or a changelog.

Low

[missing-authorization] N/A — No linked issue for this non-trivial PR. The PR introduces new structured types, changes error handling semantics, adds validators, and reduces valid output from 34 to 20 packages. The PR description and stacking on Introduce structured FBC types: MajorMinor and Date #29 provide context, but a linked issue would establish explicit authorization.
[stale-doc] docs/FBC_SCHEMA.md:37 — States "Contiguity is validated at the PLCC input layer (REQ-DATE-04)." After this PR, contiguity is also validated in the FBC filter pipeline by ValidatePhaseContiguity. The existing statement is incomplete (not false — it doesn't say "only").
[design-direction] pkg/fbc/filter.go — The DefaultFilters() comment "Invariants required by FBC consumers. These must not be relaxed." indicates intentional architectural evolution from mutation-only filters to include post-translation invariant validators. This is a reasonable defense-in-depth design. The documentation updates requested above should acknowledge this architectural decision.
[edge-case] pkg/fbc/fbc.go:127 — newPackage uses all-or-nothing semantics: if any version fails translation, all successfully-translated versions are discarded. This is defensible (treat translation errors as product-level failures) and consistent with existing PLCC validation patterns.

Labels: PR modifies FBC package types and validators; documentation is stale

fullsend-ai-review

See the review comment for full details.

fullsend-ai-review · 2026-07-01T21:06:34Z

 // Version represents an operator version with its lifecycle phases and platform compatibility.
 type Version struct {
-	Name                  string     `json:"name"`
+	Name                  MajorMinor `json:"name"`


[medium] breaking-api

Exported struct field types changed (Version.Name, Phase.StartDate/EndDate, Platform.Versions) from string types to structured types. Breaking Go API change; JSON wire format preserved. Pre-v1 module.

Suggested fix: Document the Go API breaking changes in the PR description or a changelog.

fullsend-ai-review · 2026-07-01T21:06:34Z

 	for _, v := range product.Versions {
-		pkg.Versions = append(pkg.Versions, translateVersion(v))
+		fv, err := translateVersion(v)
+		if err != nil {


[low] edge-case

newPackage uses all-or-nothing semantics for version translation. If any version fails, all successfully-translated versions are discarded. Consistent with existing PLCC validation patterns.

joelanford and others added 2 commits July 1, 2026 16:05

joelanford requested a review from a team as a code owner July 1, 2026 20:49

joelanford changed the base branch from jl/structured-fbc-types to main July 1, 2026 20:50

fullsend-ai-review Bot suggested changes Jul 1, 2026

View reviewed changes

fullsend-ai-review Bot added documentation Improvements or additions to documentation enhancement New feature or request labels Jul 1, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add post-filter FBC invariant validators#30

Add post-filter FBC invariant validators#30
joelanford wants to merge 2 commits into
mainfrom
jl/fbc-invariant-validators

joelanford commented Jul 1, 2026 •

edited

Loading

Uh oh!

fullsend-ai-review Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

fullsend-ai-review Bot commented Jul 1, 2026

Uh oh!

fullsend-ai-review Bot left a comment

Uh oh!

fullsend-ai-review Bot Jul 1, 2026

Uh oh!

fullsend-ai-review Bot Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

joelanford commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

fullsend-ai-review Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Jul 1, 2026

Review

Findings

High

Medium

Low

Uh oh!

fullsend-ai-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

joelanford commented Jul 1, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jul 1, 2026 •

edited

Loading