feat(write-path): change impact --staleness advisory + write-time skill triggers (#58 P1+P2) - #63
Conversation
Proposal 2 - change impact --staleness advisory: - new opt-in flag; default mode stays byte-for-byte fail-closed on stale snapshots, advisory mode answers from the last committed run with freshness.status 'stale-advisory', top-level recorded/current snapshot identity fields, and a never-gate limitation entry - docs/change-impact.md documents the advisory contract; FULL_HELP_TEXT updated; registered change-impact schema extended additively (inlined freshness admitting 'stale-advisory', optional identity fields) - regression coverage in dag_run: stale+default still fails, stale+advisory answers with identities, fresh+advisory equals default, bad value rejected Proposal 1 - write-path triggers and contract: - SKILL.md frontmatter now triggers on implement/refactor/fix, mid-edit blast-radius and test-selection queries, and structural rewrite planning; new 'While writing code' section prescribes session_start -> change impact --staleness advisory -> edit.ast-grep-plan preview -> edit -> session_end - AGENTS.md and README agent workflow carry the same contract - internalization records: re-sync dag_run.rs conformance digest pins Refs #58 (proposals 1-2 of 6; 3-6 remain open there)
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
* feat(edit): working-tree blast radius without a prior run (#58) `code-intel edit impact --repo-path <checkout> --changed <path>...` answers "what does this touch, and which tests should I run" from the working tree: no artifact root, no committed run, no snapshot identity, no capability envelope. Issue #58's thesis was that the pipeline contributes nothing while an agent writes code, and that this is structural. The measured reason is narrower than the issue says: since #63 the skill already carries write verbs, but the route they point at, `change impact --staleness advisory`, calls snapshot freshness unconditionally before the staleness branch. That was ~10s on this repo. #84 cut it to 0.6s; this route avoids it entirely, at 195-308ms over 833 files. Authority is the thing traded away, and the payload says so: `"authority":"none"`, `"source":"working-tree"`, and a limitations entry stating it must never gate. A test asserts the schema name appears in no admissibility, gate, index, artifact-ref or committed-evidence path, so there is nothing for a later change to start consuming by accident. It is not a second implementation. The traversal moved to a shared `impact_graph` module that `change impact` now calls too — same reverse-import walk, same test selection, same reason and confidence labels. The import heuristics are the ones evidence.native-code already publishes. Two answers about the same tree that disagreed would be worse than one answer that is late. KNOWN GAP, measured, not hidden: on this repository the graph resolves 16 of 821 import edges. `import_target`'s Rust arm returns everything between `use ` and `;`, so `use crate::a::{b, c};` yields a target with a brace group that `resolve_import` cannot map to a path, and `mod` lines are not read at all. `change impact` has the identical weakness — it shares the resolver — so this is a pre-existing limit this route makes visible rather than one it introduces. Both `resolvedImportEdges` and `unresolvedImportEdges` are in the payload for exactly that reason. Fixing the Rust arm is the next slice and changes evidence.native-code output, so it belongs in its own change. Refs #58 * fix(acceptance): re-pin the native-code source digest the acceptance report binds `orchestration/acceptance/native-code-evidence-candidate.json` carries a sourceDigest for native_code_evidence.rs, which this branch changed when it made `language` and `extract_imports` crate-visible. The language adapter acceptance gate reported provenance sourceBound=False, and the fast conformance profile failed with it. This is the seventh place in the repository that pins that one file. The others — integrations.json, four internalization records, a test constant and the launcher — were re-synced when the visibility changed; this one is under orchestration/acceptance/ and was missed because nothing links the pin sites together. A `code-intel repin` sweep that knows every pin site is the obvious fix and is not this change. Refs #58
1 participant
Implements proposals 1 and 2 of #58 — the two
smallitems that unlock the write path.Proposal 2:
change impact --staleness advisoryThe one write-shaped query used to fail-closed the moment the working tree diverged from the committed snapshot — unusable exactly when writing. Now:
--staleness advisory(opt-in): on identity mismatch, answers from the latest committed run instead of erroring;freshness.statusbecomesstale-advisory, top-levelrecordedSnapshotIdentity/currentSnapshotIdentityexpose the gap, and a limitation entry states the answer must never gate.--staleness current): byte-for-byte identical to before — fail-closed, gate semantics untouched. Fresh + advisory equals default output.docs/change-impact.mddocuments the contract;FULL_HELP_TEXTupdated; registered schemacode-intel-change-impact.v1.schema.jsonextended additively (inlined freshness admittingstale-advisory; optional identity fields) per its compatibilityPolicy.Proposal 1: write-path triggers + "While writing code" contract
skills/code-intel-pipeline/SKILL.mdfrontmatter now triggers on implement / refactor / fix, mid-edit blast-radius and test-selection questions, and structural rewrite planning — previously zero write verbs, so the skill could not fire when the task was writing code.## While writing codesection prescribes the loop:session_start→change impact --changed <paths> --staleness advisory→edit.ast-grep-planpreview (repositoryMutation=false) → edit + selected tests →session_end(fail-on-regression).AGENTS.mdand the READMEAgent 工作流section carry the same contract.Verification
dag_runsuite extended: stale+default still exits 65 with the original error; stale+advisory exits 0 withstale-advisory+ both identities; fresh+advisory equals default;--staleness eventualrejected. 8/8 pass.internalization_record40/40 (dag_run.rs conformance pins re-synced),capability_exec35/35,schema_lifecycle2/2,cargo fmt --checkclean.tests/test_skill_package.py12/12; simulated Darwin heuristic 87.5/100.Refs #58 — proposals 3–6 remain open there.