Skip to content

docs: document extract/inline architecture - #2064

Closed
vitali87 wants to merge 1 commit into
pr-split/split-pr1550-source/pr-4from
pr-split/split-pr1550-source/pr-9
Closed

vitali87 wants to merge 1 commit into
pr-split/split-pr1550-source/pr-4from
pr-split/split-pr1550-source/pr-9

Conversation

@vitali87

Copy link
Copy Markdown
Owner

Document extract/inline behavior, refusals, atomicity, and add the architecture page to mkdocs navigation.

Files changed

  • docs/architecture/extract-inline.md
  • mkdocs.yml

Diff stats

+77 additions, -0 deletions (77 LOC)

Dependencies

This PR depends on: pr-4

Dependency graph

Merge in this order:

pr-1: feat: add extract/inline public types and constants
└── pr-2: feat: analyze extract/inline scopes and transactions
    └── pr-3: feat: implement extract refactoring operation
        ├── pr-4: feat: implement inline refactoring operation
        │   ├── pr-5: feat: expose extract/inline through CLI
        │   ├── pr-6: feat: expose extract/inline through MCP
        │   │   ├── pr-8: test: cover inline, MCP, and CRLF regression
        │   │   └── pr-10: docs: list extract/inline MCP tools
        │   └── pr-9: docs: document extract/inline architecture  <-- this PR
        └── pr-7: test: cover extract refactoring
            └── pr-8: test: cover inline, MCP, and CRLF regression

@coderabbitai

coderabbitai Bot commented Sep 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)
  • ^feat/.*
  • ^fix/.*

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: vitali87/code-graph-rag/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 34c59b02-709b-40b3-8609-df482c31f199

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

Not safe to merge as documented: the new page directs users to commands and tools that the application does not expose.

Findings

  1. P1 Unavailable interfaces documented ▶

Summary

This PR adds an Architecture page for extract and inline operations and adds it to the MkDocs navigation. The page currently advertises CLI commands and MCP tools that are not registered, so its usage examples do not work in the current revision.

Reviews (1) · Last reviewed commit: "docs: document extract/inline architectu..."

Comment thread docs/architecture/extract-inline.md Resolved
@greptile-apps

greptile-apps Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Comments Outside Diff

These findings sit on lines the diff does not cover, so they could not be posted inline. Each one leaves this list once its file changes.

  • P1 Documentation advertises unregistered extract and inline interfaces ▶

    • Bug
      • docs/architecture/extract-inline.md:11-18 presents cgr extract, cgr inline, and MCP tools extract/inline as available. The checked-out CLI registration contains neither command, and neither name appears in the MCP tool enum or registry.
    • Cause
      • The extract/inline implementation exists as the library facade in codebase_rag/editing/extract.py:23-62, but no CLI command or MCP tool registration was added for it.
    • Fix
      • Either register CLI commands and MCP schemas/handlers for extract and inline, including the documented parameters, or revise/remove the availability claims in docs/architecture/extract-inline.md:11-18.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The documentation contains unresolved correctness, safety, and availability issues.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 2 Medium severity · 2 Low severity

Open (5)
What changed in this PR

Documents the extract/inline refactoring architecture and adds the page to MkDocs navigation.

Changes:

  • Documents scope analysis, substitution, refusals, transactions, atomicity, and contracts.
  • Adds the architecture page to the navigation.
File Review findings
mkdocs.yml Adds the extract/inline architecture page to navigation.
docs/​architecture/​extract-inline.md Nit (3 votes): CLI/MCP examples precede the PRs that introduce them. Nit (2 votes): Clarify single-output and no-output JavaScript/TypeScript cases. Moderate (3 votes): “Every call site” overstates best-effort rewriting, and _call_at lacks the recorded end span. Moderate (3 votes): Scope the postcondition contract to the re-ingest path or make the callback mandatory. Moderate (1 vote): Restrict docstring filtering to a leading docstring or document the current behavior. Critical (1 vote): Repeated non-atomic arguments can duplicate evaluation and side effects during inlining. Nit (1 vote): Qualify _early_exit behavior to the extracted function’s active scope. Moderate (1 vote): Document or implement refusal of parameter side effects such as x.pop(). Moderate (1 vote): Qualify the import cleanup guarantee for aliased imports.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/architecture/extract-inline.md Resolved
Comment thread docs/architecture/extract-inline.md Resolved
Comment thread docs/architecture/extract-inline.md Resolved
Comment thread docs/architecture/extract-inline.md Resolved
Comment thread docs/architecture/extract-inline.md Resolved

vitali87 commented Oct 9, 2026

Copy link
Copy Markdown
Owner Author

Closing as superseded: this is part of the split of #1550, which was closed without merging, and it targets the stale feat/move-op branch rather than main. The branch is kept, so the extract/inline work can be rebased onto main if it's picked up again.


Generated by Claude Code

@vitali87 vitali87 closed this Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants