feat: preview and confirm before injecting instructions into CLAUDE.md by lngyeen · Pull Request #204 · tirth8205/code-review-graph

lngyeen · 2026-04-10T08:51:52Z

Summary

Addresses #173 — install silently rewrites CLAUDE.md / platform rules without preview.

Three changes:

--dry-run now covers instruction injection: shows the full content that would be appended to CLAUDE.md and platform rule files, then exits without writing.
Interactive confirmation: before injecting, the installer prints the full content and list of target files, then prompts Inject into CLAUDE.md, AGENTS.md, ...? [Y/n]. Non-TTY environments (CI, piped input) auto-confirm for backwards compatibility.
New flags:
- --yes / -y: auto-confirm without prompting
- --no-instructions: skip injection entirely

Example output

Graph instructions will be appended to: CLAUDE.md, AGENTS.md (new), .cursorrules (new)
Content to inject:
  <!-- code-review-graph MCP tools -->
  ## MCP Tools: code-review-graph
  ...

Inject into CLAUDE.md, AGENTS.md (new), .cursorrules (new)? [Y/n]

Test plan

All 615 tests pass
--dry-run shows preview without writing
--no-instructions skips injection
--yes auto-confirms
Non-TTY defaults to confirm (backwards compat)
Idempotent: already-injected files are skipped

Closes #173

Three changes to address silent CLAUDE.md/platform rules rewriting: 1. --dry-run now covers instruction injection: shows full content that would be appended to CLAUDE.md and platform rule files. 2. Interactive confirmation: before injecting, the installer prints the full content and target files, then prompts for Y/n. Non-TTY environments (CI, piped input) auto-confirm for backwards compat. 3. New flags: - --yes / -y: auto-confirm without prompting - --no-instructions: skip injection entirely Closes tirth8205#173

tirth8205 · 2026-04-11T08:52:02Z

The preview-and-confirm UX is exactly what issue #173 asked for — good design choices: dry-run shows preview, --no-instructions opt-out, --yes for CI, non-TTY auto-confirms. The implementation is clean.

Two things to address before merging:

1. Logic bug: inject_claude_md not counted in 'injected' list

In this block:

inject_claude_md(repo_root)
updated = inject_platform_instructions(repo_root)
injected = ["CLAUDE.md"] + updated if (repo_root / "CLAUDE.md").exists() else updated

The conditional check is wrong due to Python operator precedence — this evaluates as:
injected = ["CLAUDE.md"] + (updated if condition else updated)
which always includes "CLAUDE.md". It should be:
injected = (["CLAUDE.md"] + updated) if already_confirmed_we_wrote_it else updated

More importantly, inject_claude_md also creates CLAUDE.md if it doesn't exist, so checking .exists() post-write isn't the right guard anyway. Simplest fix: just always include "CLAUDE.md" in the list since inject_claude_md was unconditionally called.

2. Type incompatibility with PR #142

_preview_instructions expects platform_files as dict[str, str] (label -> filename), but if PR #142 merges first, _PLATFORM_INSTRUCTION_FILES becomes dict[str, tuple[str, ...]] (filename -> owners tuple). The for loop 'for label, filename in platform_files.items()' will then unpack incorrectly. Please check for ordering dependency or make _preview_instructions robust to the updated type.

Everything else looks good — the --no-instructions skip path, the --yes flag, the dry-run preview, and the non-TTY backwards-compat default are all correct.

… (supersedes #204 #207 #179) (#228) * docs(troubleshooting): add quick-reference for the 4 most common support patterns These four issues account for the majority of support questions: 1. "Hooks use a matcher + hooks array" — users on pre-v2.2.3 releases. Answer: upgrade to v2.2.4 and re-run `code-review-graph install`. 2. "code-review-graph: command not found" after pip install — PATH issue. Answer: use pipx, uvx, or `python -m code_review_graph`. 3. "Is this project-scoped or user-scoped?" — 4-piece scope table (package is user-scoped, graph+mcp config are project-scoped, multi-repo registry is user-scoped). 4. "Built the graph but Claude Code doesn't see it in a new session" — didn't restart Claude Code / wrong cwd / ran build but not install. The new section is placed at the top of TROUBLESHOOTING.md so it's found first by anyone searching for these error messages. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add Elixir parser support (#112) Closes #112. tree-sitter-elixir is already bundled with tree-sitter-language-pack so no new runtime dep. Elixir's AST is uniform: ``defmodule``, ``def``/``defp``/``defmacro``, ``alias``/``import``/``require``/``use``, and ordinary function invocations all share the ``call`` node type and are distinguished only by the leading identifier text. Added a dedicated ``_extract_elixir_constructs`` helper (following the R and Lua pattern) that dispatches by first-identifier: - ``defmodule Name do ... end`` -> Class node, recurse into do_block with ``enclosing_class=Name`` - ``def`` / ``defp`` / ``defmacro`` / ``defmacrop`` -> Function/Test node attached to the enclosing module, recurse into do_block with ``enclosing_func=<fn>`` so nested calls resolve to qualified names - ``alias`` / ``import`` / ``require`` / ``use`` -> IMPORTS_FROM edge targeting the module name (flat or dotted, e.g. ``Foo.Bar``) - Anything else -> CALLS edge with the leading identifier as the target name, followed by recursion into arguments + do_block for nested calls Helpers added: - ``_elixir_call_identifier`` — handles both plain identifiers and ``dot > alias + identifier`` for ``IO.puts`` style calls - ``_elixir_module_name`` — extracts a module name from an arguments subtree (``alias`` node or ``dot`` node) - ``_elixir_function_name_and_params`` — extracts the function name from the inner call node inside ``def``'s arguments Verified on ``tests/fixtures/sample.ex`` (two modules with 6 functions total across public/private def/defp, plus alias/import/require): - 9 nodes, 16 edges - 2 Class nodes (Calculator, MathHelpers) - 6 Function nodes with correct parent_name - 3 IMPORTS_FROM (alias, import, require) - 5 CALLS with internal resolution: Calculator.compute -> Calculator.add, MathHelpers.double -> Calculator.compute, MathHelpers.triple -> MathHelpers.double, etc. Tests (7 new in TestElixirParsing): - Language detection (.ex + .exs) - Module -> Class mapping - def/defp producing Function nodes with correct parents - alias/import/require producing IMPORTS_FROM - Internal call resolution to qualified names - CONTAINS edges linking module -> functions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: apply_refactor_tool dry-run mode returning unified diffs (#176) Closes #176. The existing rename_preview / apply_refactor flow was technically two-step, but apply_refactor wrote to disk immediately so there was no way to inspect the exact diff before committing. apply_refactor() now takes an optional ``dry_run: bool`` parameter. When true: - Compute the same edit plan as the real write path, grouping edits by file so multiple edits to the same file apply sequentially against updated content (fixes a subtle bug where separate edits on the same file could stomp each other) - Return a unified-diff string per would-be-modified file using ``difflib.unified_diff`` - Leave the refactor_id in the pending cache so the user can review the diff then call again with ``dry_run=False`` to commit The MCP tool wrapper (``apply_refactor_tool`` in main.py + ``apply_refactor_func`` in tools/refactor_tools.py) now exposes the ``dry_run`` parameter. The real-write path was also refactored to share the plan-computation step with dry-run — multi-edit files now work correctly in both modes. Tests (2 new in test_refactor.py): - test_apply_refactor_dry_run_returns_diff_without_writing: confirms the diff contains both old and new names, the file on disk is unchanged, the refactor_id is still valid after the dry-run, and a follow-up real apply with the same id succeeds and consumes the id. - test_apply_refactor_dry_run_no_edits: empty edit list returns an empty diffs dict. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: CRG_DATA_DIR / CRG_REPO_ROOT env vars for out-of-tree graph storage (#155) Closes #155 (supersedes #207 which had unresolved review comments about input()-on-stdio and _local_only fragility). New env vars let users keep the .code-review-graph directory outside their working tree — useful for ephemeral workspaces, Docker volumes, shared CI caches, and multi-repo orchestrators where scripting the CLI from outside the repo is necessary. ## CRG_DATA_DIR When set, replaces the default ``<repo>/.code-review-graph`` directory verbatim. New ``get_data_dir(repo_root)`` helper in incremental.py is the single source of truth — it creates the directory (respecting the override) and writes the auto-generated inner ``.gitignore`` with ``*`` so graph files are never committed. Call sites updated to use ``get_data_dir()``: - ``incremental.py::get_db_path`` (the graph.db location) - ``cli.py::visualize`` (graph.html) - ``cli.py::wiki`` (wiki output dir) - ``tools/docs.py::generate_wiki_func`` (MCP wiki tool) - ``tools/docs.py::get_wiki_page_func`` (MCP wiki lookup) The legacy ``<repo>/.code-review-graph.db`` migration is preserved but only runs when CRG_DATA_DIR is unset (there's no relationship between the legacy location and an override location). ## CRG_REPO_ROOT ``find_project_root()`` now checks ``CRG_REPO_ROOT`` before the usual git-root walk. Useful for anyone scripting ``code-review-graph`` from a cwd that isn't inside the target repo (daemons, CI jobs, wrappers). Falls through to normal resolution if the path doesn't exist. ## Deliberate design choices (vs PR #207) - **No input() / confirmation prompts** — this branch of code runs from the CLI entry and can also be pulled into MCP wrappers which pipe stdio. Reading from stdin there would corrupt JSON-RPC. - **No ``mcp._local_only`` attribute** — that approach was fragile because it touched a private FastMCP attribute. - **Tilde expansion** — both env vars go through ``Path.expanduser()`` so ``CRG_DATA_DIR=~/Library/crg`` works. Tests (5 new in TestDataDir): - test_default_uses_repo_subdir — unset env, default behavior - test_env_override_replaces_repo_subdir — CRG_DATA_DIR works - test_get_db_path_uses_data_dir — graph.db follows the override - test_find_project_root_env_override — CRG_REPO_ROOT works - test_find_project_root_env_override_missing_dir_falls_through — bogus override doesn't crash, falls back to git root Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: stderr warning before cloud embedding providers exfiltrate code (#174) Closes #174 (supersedes #179 which overlapped with #207 and used input()-on-stdio that would corrupt MCP transport). ``get_provider()`` now prints an explicit warning to **stderr** (never stdout/stdin) before returning a Google Gemini or MiniMax provider: ⚠️ code-review-graph: about to embed code via the 'google' cloud provider. Your source code (function names, docstrings, file paths) will be sent to an external API. To skip this warning in future runs, set CRG_ACCEPT_CLOUD_EMBEDDINGS=1 in your environment. To stay fully offline, use the default 'local' provider instead (no API key needed). Why stderr: the MCP stdio transport uses stdout for JSON-RPC. Writing to stdout or reading from stdin would corrupt the transport. stderr is safe and is captured by Claude Code's MCP logs. Why a warning not a block: the user already explicitly chose a cloud provider (either via ``provider="google"`` arg or via the MiniMax API key + explicit provider selection). Blocking would be paternalistic; warning is informative. The ``CRG_ACCEPT_CLOUD_EMBEDDINGS=1`` env var lets CI / scripted workflows acknowledge once and move on without repeated noise. Tests (4 new in TestCloudProviderWarning): - test_minimax_triggers_stderr_warning — captures stderr, asserts warning content, asserts stdout stays clean - test_google_triggers_stderr_warning — same for google - test_accept_env_var_suppresses_warning — CRG_ACCEPT_CLOUD_EMBEDDINGS=1 silences the warning - test_local_provider_never_warns — offline path stays quiet Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

tirth8205 · 2026-04-11T20:46:45Z

Thank you for this — closing as superseded by PR #228, which is a clean reimplementation of your changes on top of current `main` (v2.2.4 + the features from #227). The bundle ships in v2.3.0.

Your original design + issue discussion pushed this forward; really appreciate the work. If there's anything in the reimplementation you think is missing or wrong, please open a follow-up issue and I'll address it quickly.

@implementation

Additive feature release on top of v2.2.4 — new languages, new platform, new env vars, new MCP tool parameters. No breaking changes. New: - #112 Elixir parser (.ex/.exs) — modules, def/defp/defmacro, alias/import/require/use, internal call resolution - #88 Objective-C parser (.m) — @interface/@implementation, message expressions, C-style main, #import - #197 Bash/Shell parser (.sh/.bash/.zsh) — functions, commands, source imports with path resolution - #83 Qwen Code as a supported MCP install platform - #176 apply_refactor_tool dry-run mode (returns unified diffs without writing, keeps refactor_id valid for follow-up) - #155 CRG_DATA_DIR / CRG_REPO_ROOT env vars for out-of-tree graph storage and scripting from outside the repo - #173 install preview + --no-instructions + -y/--yes (supersedes closed PR #204) - #174 cloud embeddings stderr warning + CRG_ACCEPT_CLOUD_EMBEDDINGS opt-out (supersedes closed PR #179) Fixed: - Multi-edit refactor correctness bug (same-file edits could stomp each other) Docs: - TROUBLESHOOTING.md quick-reference section for the 4 most common support questions (hooks schema error, command not found, scoping, "built but not visible") Verified locally on Python 3.11: - ruff clean, mypy clean, bandit clean - 735 tests pass (+18 vs v2.2.4), coverage 74.63% - 6-repo smoke test still passes end-to-end on express/fastapi/flask/ gin/httpx/next.js Supersedes closed PRs: - #204 (install preview, credit @lngyeen) - #207 (CRG_DATA_DIR, credit @yashmewada9618) - #179 (cloud embeddings warning, credit @Bakul2006) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@implementation

Additive feature release on top of v2.2.4 — new languages, new platform, new env vars, new MCP tool parameters. No breaking changes. New: - #112 Elixir parser (.ex/.exs) — modules, def/defp/defmacro, alias/import/require/use, internal call resolution - #88 Objective-C parser (.m) — @interface/@implementation, message expressions, C-style main, #import - #197 Bash/Shell parser (.sh/.bash/.zsh) — functions, commands, source imports with path resolution - #83 Qwen Code as a supported MCP install platform - #176 apply_refactor_tool dry-run mode (returns unified diffs without writing, keeps refactor_id valid for follow-up) - #155 CRG_DATA_DIR / CRG_REPO_ROOT env vars for out-of-tree graph storage and scripting from outside the repo - #173 install preview + --no-instructions + -y/--yes (supersedes closed PR #204) - #174 cloud embeddings stderr warning + CRG_ACCEPT_CLOUD_EMBEDDINGS opt-out (supersedes closed PR #179) Fixed: - Multi-edit refactor correctness bug (same-file edits could stomp each other) Docs: - TROUBLESHOOTING.md quick-reference section for the 4 most common support questions (hooks schema error, command not found, scoping, "built but not visible") Verified locally on Python 3.11: - ruff clean, mypy clean, bandit clean - 735 tests pass (+18 vs v2.2.4), coverage 74.63% - 6-repo smoke test still passes end-to-end on express/fastapi/flask/ gin/httpx/next.js Supersedes closed PRs: - #204 (install preview, credit @lngyeen) - #207 (CRG_DATA_DIR, credit @yashmewada9618) - #179 (cloud embeddings warning, credit @Bakul2006) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

tirth8205 mentioned this pull request Apr 11, 2026

fix: --platform target now filters skills, hooks, and instruction files (#133) #142

Merged

This was referenced Apr 11, 2026

feat: qwen-code platform + Objective-C + Bash parser support #227

Merged

feat: Elixir + refactor dry-run + CRG_DATA_DIR + cloud warning + docs (supersedes #204 #207 #179) #228

Merged

tirth8205 closed this Apr 11, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: preview and confirm before injecting instructions into CLAUDE.md#204

feat: preview and confirm before injecting instructions into CLAUDE.md#204
lngyeen wants to merge 1 commit intotirth8205:mainfrom
lngyeen:fix/no-instructions-flag

lngyeen commented Apr 10, 2026

Uh oh!

tirth8205 commented Apr 11, 2026

Uh oh!

tirth8205 commented Apr 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

lngyeen commented Apr 10, 2026

Summary

Example output

Test plan

Uh oh!

tirth8205 commented Apr 11, 2026

Uh oh!

tirth8205 commented Apr 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants