Repository navigation
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: vitali87/code-graph-rag/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| Old parameters that no spec names are dropped; the values sites passed for | ||
| them are dropped with them. |
There was a problem hiding this comment.
The guide says values supplied for omitted old parameters are dropped. The mapper instead treats a remaining supplied value as a surplus argument and leaves that call site unchanged. This is a non-blocking documentation concern, but users may otherwise expect incompatible calls to be rewritten automatically.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| All edits are span-preserving [patcher](patchers.md) edits staged in one | ||
| [edit transaction](edit-transactions.md); a file that no longer parses rolls | ||
| the whole change back. Applied changes are recorded for `cgr edits undo` | ||
| and held to the [postcondition contract](postcondition-contract.md): every | ||
| call site of the changed signature must be mapped (a rewritten site is | ||
| mapped by construction) or listed as unmapped, no `too_many` finding may | ||
| remain unlisted, no site resolved by guesswork may have been rewritten | ||
| without leave, and no duplicate group or import cycle may appear. A failing | ||
| contract undoes the transaction and re-ingests the restored files; the | ||
| verdict, with the tests reaching the changed symbols, rides on the report. |
There was a problem hiding this comment.
The guide promises postcondition enforcement and a verdict containing affected tests for every applied change. Without a re-ingest callback, including for an unindexed MCP project, an edit can apply without contract enforcement and return no verdict. This is a non-blocking documentation concern, but users may rely on validation and rollback guarantees that are unavailable in that configuration.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The new architecture doc currently states that arguments for dropped parameters are automatically dropped, but the implementation leaves such sites unmapped (surplus args), so the docs are inaccurate.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
Adds documentation for the new change_signature edit operation, including an architecture deep-dive, an MCP tool table entry, and site navigation to surface the new page in the docs.
Changes:
- Add a new architecture guide page describing
change_signaturemapping, rewrite behavior, type checks, and contract/atomicity. - Add the
change_signaturerow to the MCP server “Available Tools” table. - Add a “Change Signature” entry to MkDocs navigation under Architecture.
| File | Description |
|---|---|
| mkdocs.yml | Adds nav entry to expose the new architecture page. |
| docs/guide/mcp-server.md | Adds the change_signature tool description to the generated MCP tools table section. |
| docs/architecture/change-signature.md | New architecture documentation for how change-signature planning/rewrites/contracts work. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| Old parameters that no spec names are dropped; the values sites passed for | ||
| them are dropped with them. |
| | `importers` | Modules that import a module, with each import statement's line, column, bound alias and imported symbol. Deterministic: fixed graph queries, no LLM, same graph gives the same JSON. Use this instead of query_code_graph whenever you know the exact name or location. | | ||
| | `tests_reaching` | Test functions and methods from which a qualified name is reachable through CALLS / REFERENCES / INSTANTIATES, with the distance and the symbol each test reaches it through: what to run after editing it. Deterministic: fixed graph queries, no LLM, same graph gives the same JSON. Use this instead of query_code_graph whenever you know the exact name or location. | | ||
| | `rename` | Rename a function, method, class or other definition everywhere the graph knows it is referenced: the definition, every call, reference and instantiation site, import statements (aliases kept), overrides in both directions, and Python `__all__` entries. Refuses when any site was resolved heuristically, by overload fan-out, or only by a trace, unless `allow_heuristic` is true. A class referenced by an inheritance or type annotation edge that carries no rewrite location refuses regardless of `allow_heuristic`. Every rewritten file must still parse; otherwise nothing is written. Set `dry_run` to see the plan and diff without touching the tree. Applied edits are recorded for `cgr edits undo`, and Markdown mentions of the old name are listed for a human to review. | | ||
| | `change_signature` | Change a function's or method's parameter list and rewrite every call site the graph knows per an explicit mapping. `new_params` lists the new parameters in order as `name[:annotation][=default][@source]`: `@2` takes the value from old positional index 2, `@old` from the old parameter `old`, `=literal` without a source inserts that literal where a site passes nothing (and becomes the definition's default), and a bare name is unmapped: sites passing no value for it are left untouched and listed. Overrides in the hierarchy are rewritten too; sites resolved by guesswork are listed as unmapped unless `allow_heuristic`. A default literal that does not fit the declared type is refused. Applied through one transaction and held to the postcondition contract; set `dry_run` to see the plan. | |
22073fa to
519805e
Compare
Architecture guide, MCP guide row, and navigation entry for the new change-signature operation.
Files changed
docs/architecture/change-signature.mddocs/guide/mcp-server.mdmkdocs.ymlDiff stats
+80 additions, -0 deletions (80 LOC)
Dependencies
This PR depends on:
pr-6Dependency graph
Merge in this order: