feat(impact): add deterministic blast-radius library - #26
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis change adds impact report construction from structural diffs and before/after context graphs. It defines report models and errors, validates inputs, traces artifact and package impacts, records unresolved boundaries, normalizes results, and adds comprehensive tests. ChangesImpact report
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant StructuralDiffReport
participant ContextGraphReport
participant build_impact_report
participant ArtifactImpactTraversal
participant PackageImpactTraversal
participant ImpactReport
StructuralDiffReport->>build_impact_report: provide structural changes
ContextGraphReport->>build_impact_report: provide before and after graphs
build_impact_report->>ArtifactImpactTraversal: trace artifact references
ArtifactImpactTraversal->>PackageImpactTraversal: trace package dependencies
PackageImpactTraversal->>build_impact_report: return package impacts
ArtifactImpactTraversal->>build_impact_report: return artifact impacts and boundaries
build_impact_report->>ImpactReport: assemble normalized report
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
ⓘ Your Qodo trial ends soon. Ask your workspace admin to set up billing to keep reviews running after the trial. Manage billing |
TheHalfMoon
marked this pull request as ready for review
August 26, 2026 06:58
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
PR Summary by QodoAdd deterministic blast-radius impact analysis library
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
1. Identityless seeds are emitted
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+104
to
+105
| if diff.schema != StructuralDiffReport::SCHEMA_V1 { | ||
| return Err(ImpactError::UnsupportedDiffSchema { found: diff.schema }); |
There was a problem hiding this comment.
1. build_impact_report errors untested 📘 Rule violation ▣ Testability
The new public impact handler implements multiple explicit failure branches, but every added test invokes it with valid inputs and unwraps success. Unsupported schemas, missing or ambiguous subjects/artifacts, conflicting filenames, and inconsistent resolved references therefore have no automated assertions for their error type or message.
Agent Prompt
## Issue description
Add deterministic automated coverage for every explicit `ImpactError` branch returned by `build_impact_report`.
## Issue Context
The current impact tests exercise only successful report construction. Tests should force unsupported diff/context/lock schemas, missing and duplicated subject packages, missing and duplicated artifacts, conflicting resource filenames, and resolved references with a candidate count other than one, then assert the exact error variant and stable message.
## Fix Focus Areas
- crates/commandf-pkg/src/impact.rs[99-162]
- crates/commandf-pkg/src/impact.rs[292-340]
- crates/commandf-pkg/src/impact.rs[382-400]
- crates/commandf-pkg/tests/impact.rs[9-330]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+220
to
+224
| match (before_canonical, after_canonical) { | ||
| (None, None) => { | ||
| if resource.kind == ResourceKeyKind::Canonical { | ||
| push_seed_from_key(&mut seeds, &resource, &state, before, after); | ||
| } |
There was a problem hiding this comment.
2. Identityless seeds are emitted 🐞 Bug ≡ Correctness
When a canonical diff entry omits both optional filenames, build_seeds emits an ImpactSeed with neither a before nor after artifact identity, so the reported change cannot be traversed or tied to exact digest evidence. CF-12 requires missing required seed identity to fail closed (or be explicitly marked unsupported), but this schema has no unsupported state and silently returns a successful report.
Agent Prompt
## Issue description
Reject malformed structural-diff seed evidence instead of emitting a canonical seed with no exact artifact identity.
## Issue Context
StructuralDiffReport filenames are optional, and `artifact_for_filename` maps an absent filename to `None`. CF-12 requires missing required identity evidence to fail closed unless an explicit unsupported state is represented; the current model has no such state.
## Fix Focus Areas
- crates/commandf-pkg/src/impact.rs[174-265]
- crates/commandf-pkg/src/impact_error.rs[4-41]
- crates/commandf-pkg/tests/impact.rs[133-246]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+107
to
+110
| for (side, graph) in [("before", before_graph), ("after", after_graph)] { | ||
| if graph.schema != ContextGraphReport::SCHEMA_V1 { | ||
| return Err(ImpactError::UnsupportedContextSchema { | ||
| side, |
There was a problem hiding this comment.
3. Dangling edges create phantom impacts 🐞 Bug ≡ Correctness
validate_inputs checks only schema numbers, then artifact and package traversal trusts edge endpoint identities without verifying that they exist in the graph's node collections. A schema-compatible graph with a dangling resolved source or dependency from endpoint therefore produces an impact relation for a nonexistent artifact or package instead of failing closed.
Agent Prompt
## Issue description
Validate Context Graph node and edge invariants before computing impact so dangling identities cannot become reported impacts.
## Issue Context
The context builder normally creates edges from known nodes, but `build_impact_report` is a public API accepting public, deserializable report structs. The CF-12 contract requires malformed required evidence to fail closed.
## Fix Focus Areas
- crates/commandf-pkg/src/impact.rs[99-121]
- crates/commandf-pkg/src/impact.rs[382-501]
- crates/commandf-pkg/src/impact_error.rs[4-41]
- crates/commandf-pkg/tests/impact.rs[9-330]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
1 participant
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.
Stack
CF-12 implementation Stack A for tasks T010-T018.
Base: canonical CF-12 planning merge
cefa5e4a56041bf88e833844a318b170e7e7ae83.Summary
Add the library-owned deterministic impact-analysis contract and traversal engine without shipping the CLI yet.
Changes
resolvedcanonical-reference edges;externalandambiguousedges as explicit unresolved boundaries without traversing them;bothonly for exact identical relations;Authority boundary
This PR does not:
commandf impactCLI yet;Required qualification
Keep Draft until the exact head passes mandatory workspace gates and all applicable repository workflows, and every substantive returned review finding is dispositioned.
Stack B will ship the user-visible CLI and
cf12-impact-proofonly after this library contract is qualified.Summary by CodeRabbit
New Features
Tests