docs: refresh design doc to current architecture#348
Open
jcogilvie wants to merge 1 commit into
Open
Conversation
jcogilvie
force-pushed
the
doc-update
branch
4 times, most recently
from
8d1f609 to
a10c7c7
Compare
The design doc had drifted significantly from the implementation. This brings every section, interface block, and diagram in sync with the current cmd/diff/ tree, and adds guidance to CLAUDE.md so future code changes prompt corresponding doc updates. Highlights: - Document the comp subcommand and CompDiffProcessor (entirely missing). - Rewrite all interface blocks in §6 to current signatures, add FunctionProvider, CompDiffRenderer, and structured-output types. - Replace the obsolete render.Outputs / RenderWithRequirements narrative with the upstream CompositionOutputs / RenderToStableState reality, including --eventual-state mode. - Cover nested-XR recursion, the two-phase diff calculation, claim-with- spec.claimRef synthesis, and Cleanup semantics. - Drop §10.15 (--namespace flag is complete: xr reads namespace from YAML; comp's -n filter has shipped). - Add §4 test-case categories for comp, structured output, nested XRs, eventual state, and claim edge cases. - Update §12 layout and §8 CLI examples for xr / comp / --output / etc. - Refresh all 10 mermaid sources (and regenerate SVGs via the minlag/mermaid-cli Docker image). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
Contributor
There was a problem hiding this comment.
⚠️ Not ready to approve
Multiple updated mermaid diagrams and a design-doc section still describe structured output and interface signatures that don’t match the current Go implementation.
Pull request overview
This PR refreshes the CLI diff design documentation and related diagrams to reflect the current cmd/diff/ architecture, and expands user-facing docs (README) plus contributor guidance (CLAUDE) to help prevent future doc drift.
Changes:
- Adds a detailed README section describing schema-validation failures (stderr behavior, exit code 2, and structured
errors[]/validationFailures[]schema). - Updates the design doc and mermaid diagrams to cover the
compsubcommand, nested XR recursion / eventual-state behavior, structured output, and revised interfaces. - Adds “Keeping Documentation in Sync” and PR/commit conventions guidance to
CLAUDE.md, including a diagram regeneration recipe.
File summaries
| File | Description |
|---|---|
| README.md | Documents validation-error behavior and the structured OutputError schema with examples. |
| design/design-doc-cli-diff/package-overview-fixed.mermaid | Updates high-level class diagram for new components and signatures. |
| design/design-doc-cli-diff/loader-validator-architecture.mermaid | Extends validator diagram to include scope-constraint validation. |
| design/design-doc-cli-diff/layered-architecture.mermaid | Updates layered architecture to include CompDiffProcessor, function provider, and new clients. |
| design/design-doc-cli-diff/diff-rendering-architecture.mermaid | Updates rendering diagram to include structured output and comp rendering. |
| design/design-doc-cli-diff/diff-processor-architecture.mermaid | Updates processor diagram for comp diff and two-phase diffing. |
| design/design-doc-cli-diff/diff-data-flow.svg | Regenerated diagram asset to match updated mermaid source. |
| design/design-doc-cli-diff/diff-call-sequence.mermaid | Updates call-sequence diagram to reflect xr vs comp flows and eventual-state. |
| design/design-doc-cli-diff/conceptual-layers.mermaid | Updates conceptual layer notes to include comp diff and new components/clients. |
| design/design-doc-cli-diff/client-architecture.mermaid | Adds CompositionRevision and Credential clients to the diagram. |
| design/design-doc-cli-diff/clear-layered-architecture.svg | Regenerated diagram asset to match updated mermaid source. |
| design/design-doc-cli-diff/clear-layered-architecture.mermaid | Updates simplified architecture diagram to include new components. |
| design/design-doc-cli-diff.md | Major refresh of design doc narrative, workflows, and interface blocks. |
| CLAUDE.md | Adds documentation-sync triggers and PR/commit conventions; includes a diagram regeneration recipe. |
Copilot's findings
- Files reviewed: 12/22 changed files
- Comments generated: 9
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+54
to
+57
| +CalculateDiff(ctx, composite, desired) (Diff, error) | ||
| +CalculateDiffs(ctx, xr, desired) (map, error) | ||
| +CalculateNonRemovalDiffs(ctx, xr, parent, desired) (map, set, error) | ||
| +CalculateRemovedResourceDiffs(ctx, xr, rendered) (map, error) |
| +Cleanup(ctx) error | ||
| +PerformDiff(ctx, resources, compositionProvider) (bool, error) | ||
| +DiffSingleResource(ctx, res, compositionProvider) (map, error) | ||
| +RenderToStableState(ctx, xr, comp, fns, id, observed, synthReady) (Outputs, error) |
Comment on lines
+83
to
+86
| +CalculateDiff(ctx, composite, desired) (Diff, error) | ||
| +CalculateDiffs(ctx, xr, desired) (map, error) | ||
| +CalculateNonRemovalDiffs(ctx, xr, parent, desired) (map, set, error) | ||
| +CalculateRemovedResourceDiffs(ctx, xr, rendered) (map, error) |
Comment on lines
+61
to
+63
| +Diffs map | ||
| +Summary Summary | ||
| +Errors OutputError slice |
Comment on lines
+88
to
+90
| XRStatusChanged | ||
| XRStatusUnchanged | ||
| XRStatusErrored |
Comment on lines
+94
to
+97
| +Total int | ||
| +Changed int | ||
| +Unchanged int | ||
| +Errored int |
Comment on lines
+67
to
+70
| +CompositionDiff *CompositionDiff | ||
| +AffectedResources AffectedResourcesSummary | ||
| +XRImpacts []XRImpact | ||
| +Errors []OutputError |
Comment on lines
+763
to
+781
| The structured types (in `cmd/diff/renderer/types/`) are part of the public output contract: | ||
|
|
||
| - `StructuredDiffOutput` — XR diff: per-resource diffs with field-level old/new, summary counts, errors. | ||
| - `CompDiffOutput` — composition diff: top-level `CompositionDiff`, list of `XRImpact`, `AffectedResourcesSummary`, | ||
| `DownstreamChanges`. | ||
| - `XRImpact`, `XRStatus`, `DownstreamChanges`, `AffectedResourcesSummary` — finer-grained components used by the test | ||
| harness for field-level assertions. | ||
| - `OutputError` — error envelope used by both XR and comp diff outputs. Carries: | ||
| - `ResourceID`: which user-supplied input the diff was processing (one entry per batched run) | ||
| - `Message`: human-readable error string | ||
| - `ValidationFailures`: optional `[]ResourceValidationFailure`, populated when the error originated from schema | ||
| validation. Lets machine consumers inspect typed failures without parsing `Message`. | ||
| - `ResourceValidationFailure` — per-resource view inside `ValidationFailures`. Mirrors upstream | ||
| `pkg/validate.ResourceValidationResult` (apiVersion / kind / name / namespace / status), but is owned by | ||
| `crossplane-diff` so the public JSON schema can evolve independently of upstream's. `Status` surfaces `"invalid"` and | ||
| `"missingSchema"`; valid entries are filtered out so consumers iterating `ValidationFailures` see only failure rows. | ||
| - `FieldValidationError` — single field-level error inside `ResourceValidationFailure.Errors`. Carries `Type` | ||
| (`"schema"` / `"cel"` / `"unknownField"` / `"defaulting"`), `Field` (JSONPath, when locatable), `Message`, and | ||
| `Value` (typed: string, number, bool, or struct). |
Comment on lines
+405
to
+413
| mermaid file, regenerate its `.svg` so the rendered doc matches. Prefer the official mermaid-cli Docker image for | ||
| portability across machines and CI: | ||
|
|
||
| ```bash | ||
| cd design/design-doc-cli-diff | ||
| for f in *.mermaid; do | ||
| docker run --rm -u "$(id -u):$(id -g)" -v "$PWD:/data" minlag/mermaid-cli \ | ||
| -i "/data/$f" -o "/data/${f%.mermaid}.svg" | ||
| done |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of your changes
The design doc at
design/design-doc-cli-diff.mdhad drifted significantly from the current implementation — most interface signatures were stale, thecompsubcommand and its entire processor were undocumented, and therender.Outputsnarrative referred to a struct that has since been renamed and restructured upstream. This PR brings every section, interface block, and diagram in sync with the currentcmd/diff/tree.It also adds a "Keeping Documentation in Sync" section and a "Git, Commits, Issues, and Pull Requests" section to
CLAUDE.mdso future code changes prompt corresponding doc updates and PRs follow project conventions (DCO sign-off, draft PRs, template compliance). The mermaid SVG regeneration recipe points at theminlag/mermaid-cliDocker image for portability.What's new in the doc:
compsubcommand andCompDiffProcessor(entirely missing).FunctionProvider,CompDiffRenderer, and the structured-output types.render.Outputs/RenderWithRequirementsnarrative with the upstreamCompositionOutputs/RenderToStableStatereality, including--eventual-statemode.spec.claimRefsynthesis, andCleanupsemantics.--namespaceflag is complete:xrreads namespace from YAML;comp's-nfilter has shipped).comp, structured output, nested XRs, eventual state, and claim edge cases.xr/comp/--output/ etc.I have:
RunDocs-only change; no code paths exercised.earthly -P +reviewableto ensure this PR is ready for review.Added or updated unit tests.Docs-only change.Added or updated e2e tests.Docs-only change.Followed the API promotion workflow if this PR introduces, removes, or promotes an API.No API changes.