Repository navigation
feat(projection): per-target change classification on check and plan (#327 S4) - #347
Merged
Merged
Conversation
… and plan results ProjectionTargetChangeV1 and ProjectionResultV2.changes: one entry per target path with stamp, output, semantic and proof comparisons (null when unknown), the planned file action and a classification derived by one precedence rule (blocked > baseline-missing > proof-change > semantic-change > output-change > stamp-only > unchanged). The validators bind plannedAction to files[], blocked targets to coverage, and keep changes off apply results and receipts. The result schema and its embedded readback copy carry the same targetChange definition. Refs #327
…esults check and plan results carry changes[]: one entry per target path over the current layout, the committed manifest rows and the orphans. The comparisons reuse what the render holds (row stamps and output digests, per-capability semantic and flow-proof fingerprints, selector-evidence digests); plannedAction is the result's own files[] action, so the targets with a planned action are exactly the documents apply writes or deletes. Phase 1 carried-forward targets, review orphans and adoption or ownership rejections are blocked. A source-only edit is stamp-only with only the manifest written; a missing baseline is baseline-missing, never unchanged. Status, files, apply, receipts and freshness are unchanged. Refs #327
Runbook: the changes[] fields, the classification table and precedence, plannedAction semantics, the baseline-missing state, and how a consumer ignores stamp-only drift. ADR-0047 section 7 records the decision. Refs #327
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f882d810f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A global target (index, decisions index, changelog, diagrams, relation summaries) renders the whole model, including nodes no capability owns. Comparing only capability fingerprints left such targets unchanged when a module or system node outside every capability moved. They now also compare the committed semanticBaseline.modelDigest with the current model digest and report semantic-change with the new model-digest-changed reason code. Module documents keep their per-capability comparison, and a source-only edit never moves the model digest. Refs #327
…ry file action in changes A path rejected for adoption or ownership repair keeps plannedAction none and stays blocked, but its files[] update proves the on-disk bytes differ from the proposed ones. outputChanged and file-bytes-changed now follow the underlying files[] action instead of the blocked write, and the contract requires outputChanged whenever the path's files[] entry changes a file. The contract also checks the reverse binding: every non-manifest files[] create, update or delete, rejected paths included, has a change of its path, and no change names the projection manifest. The manifest path constant moves next to the result contract that now reads it. Refs #327
…e action The protocol promises changes on every check and plan result, but the JSON schema cannot condition on the request mode, so the mode validator now reports a check or plan result without changes. Every producer path already builds its result through projectionProtocolResult, which classifies for check and plan and runs this validator before returning. The change invariants also refuse a change that names the projection manifest. Tests cover a missing change for a non-manifest create, update or delete, the mode check per status, and end to end a model change outside every capability and an adoption-required edited index whose blocked change keeps file-bytes-changed. The runbook and ADR-0047 describe both bindings. Refs #327
…he classification fixes Refs #327
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.
Refs #327 (slice S4, finding 2: stamp noise). Does not close the issue on its own.
Feature flag:
projection-change-classification-v1is inARCHCTX_FEATURES, the capabilities fixture and the schema (commit 656e6f4, added after #337 merged;archctx capabilities --jsonlists it). A consumer detectschanges[]through that flag.What changes
projection runcheck and plan results add a per-targetchanges[]:{targetId, path, affectedCapabilityIds, stampChanged, outputChanged, semanticChanged, proofChanged, plannedAction, classification, reasonCodes}Classification order. Each target gets the first class that applies:
blockedproof-change, when the proof change is knownbaseline-missingsemantic-changeoutput-changestamp-onlyunchangedWhen a target is
stamp-only. All of these must hold:A missing baseline or fingerprint is
nullorbaseline-missing. It is never reported asunchanged.What each target will write.
plannedAction(none|create|update|delete) comes from the same byte decision apply uses (projectionWriteFileResults). There is no second rule. Underplannedorplanned-partial, the targets with a planned action are exactly the documents apply writes. The manifest is never achanges[]entry; its write is the onefiles[]entry no change names.Which targets are listed.
changes[]covers current targets and committed-manifest targets, so additions and orphan deletes are included. An orphan that needs review isblockedwith no action.Where
blockedcomes from. It is derived from S7 isolation, review orphans and adoption/ownership rejections. The validator checks that everycoverage.blockedTargetsentry isblockedwith the same reason codes. A rejected adoption target isblocked/nonewhilefiles[]still lists it asupdate, because only an approved adoption writes it. That is the one allowed mismatch.Global documents (index, decisions index, changelog, diagrams, relation summaries) depend on the whole model. They are never
stamp-only, and a source-only edit never touches them. Any model semantic change marks themsemantic-change, so they may over-report.Unchanged:
staleNodes,projection freshness(it does not classify), apply semantics and receipts.Docs: a runbook section on telling stamp-only drift from real changes, and ADR-0047 §7.
Verification
tests/: 4 passpackaged-cli-smoke: OKpackage-boundary-audit: passedReal run on a scratch clone with a CodeGraph index:
stamp-only/nonefor that capability's module doc, andfilesholds only the manifest.output-change/update, andfilesholds the manifest plus that doc.docs driftis ok, check isnoop, and all 7 changes areunchanged.The new tests failed on afeb0c3 before the implementation. They cover: