diff --git a/code_review_graph/hints.py b/code_review_graph/hints.py index 9fd8ce59c..589699626 100644 --- a/code_review_graph/hints.py +++ b/code_review_graph/hints.py @@ -12,153 +12,169 @@ from collections import deque from typing import Any -# ---- intent categories and their characteristic tool names ---- +# ---- intent categories and their characteristic public MCP tool names ---- +# Keep these keyed on exported MCP tool names only. Do not substitute +# internal helper names from implementation modules here. _INTENT_TOOLS: dict[str, set[str]] = { "reviewing": { - "detect_changes", "get_review_context", "get_affected_flows", "get_impact_radius", + "detect_changes_tool", "get_review_context_tool", "get_affected_flows_tool", "get_impact_radius_tool", }, "debugging": { - "query_graph", "get_flow", "semantic_search_nodes", + "query_graph_tool", "get_flow_tool", "semantic_search_nodes_tool", }, "refactoring": { - "refactor", "find_dead_code", "suggest_refactorings", + "refactor_tool", "apply_refactor_tool", }, "exploring": { - "list_communities", "get_architecture_overview", "list_flows", "list_graph_stats", + "list_communities_tool", "get_architecture_overview_tool", "list_flows_tool", "list_graph_stats_tool", }, } # ---- workflow adjacency: for each tool, which tools are useful next ---- _WORKFLOW: dict[str, list[dict[str, str]]] = { - "list_flows": [ + "list_flows_tool": [ { - "tool": "get_flow", + "tool": "get_flow_tool", "suggestion": "Drill into a specific flow for step-by-step details", }, { - "tool": "get_affected_flows", + "tool": "get_affected_flows_tool", "suggestion": "Check which flows are affected by recent changes", }, { - "tool": "get_architecture_overview", + "tool": "get_architecture_overview_tool", "suggestion": "See the high-level architecture", }, ], - "get_flow": [ + "get_flow_tool": [ { - "tool": "query_graph", + "tool": "query_graph_tool", "suggestion": "Inspect callers/callees of a step in this flow", }, { - "tool": "get_affected_flows", + "tool": "get_affected_flows_tool", "suggestion": "Check if changes affect this flow", }, { - "tool": "list_flows", + "tool": "list_flows_tool", "suggestion": "Browse other execution flows", }, ], - "get_affected_flows": [ + "get_affected_flows_tool": [ { - "tool": "detect_changes", + "tool": "detect_changes_tool", "suggestion": "Get risk-scored change analysis", }, { - "tool": "get_flow", + "tool": "get_flow_tool", "suggestion": "Inspect a specific affected flow", }, { - "tool": "get_review_context", + "tool": "get_review_context_tool", "suggestion": "Build a full review context for the changes", }, ], - "list_communities": [ + "list_communities_tool": [ { - "tool": "get_community", + "tool": "get_community_tool", "suggestion": "Inspect a specific community's members", }, { - "tool": "get_architecture_overview", + "tool": "get_architecture_overview_tool", "suggestion": "See cross-community coupling and warnings", }, { - "tool": "list_flows", + "tool": "list_flows_tool", "suggestion": "See execution flows across communities", }, ], - "get_community": [ + "get_community_tool": [ { - "tool": "query_graph", + "tool": "query_graph_tool", "suggestion": "Explore callers/callees of community members", }, { - "tool": "list_communities", + "tool": "list_communities_tool", "suggestion": "Browse other communities", }, { - "tool": "get_architecture_overview", + "tool": "get_architecture_overview_tool", "suggestion": "See how this community fits the architecture", }, ], - "get_architecture_overview": [ + "get_architecture_overview_tool": [ { - "tool": "list_communities", + "tool": "list_communities_tool", "suggestion": "Drill into individual communities", }, { - "tool": "detect_changes", + "tool": "detect_changes_tool", "suggestion": "See how recent changes affect the architecture", }, { - "tool": "list_flows", + "tool": "list_flows_tool", "suggestion": "Explore execution flows", }, ], - "detect_changes": [ + "detect_changes_tool": [ { - "tool": "get_review_context", + "tool": "get_review_context_tool", "suggestion": "Build a full review context with source snippets", }, { - "tool": "get_affected_flows", + "tool": "get_affected_flows_tool", "suggestion": "See which execution flows are affected", }, { - "tool": "get_impact_radius", + "tool": "get_impact_radius_tool", "suggestion": "Expand the blast radius analysis", }, { - "tool": "refactor", + "tool": "refactor_tool", "suggestion": "Look for refactoring opportunities in changed code", }, ], - "refactor": [ + "refactor_tool": [ { - "tool": "query_graph", + "tool": "apply_refactor_tool", + "suggestion": "Apply a reviewed rename preview", + }, + { + "tool": "semantic_search_nodes_tool", + "suggestion": "Find related symbols to also rename", + }, + { + "tool": "query_graph_tool", "suggestion": "Verify call sites before applying a rename", }, + ], + "apply_refactor_tool": [ { - "tool": "detect_changes", - "suggestion": "Check risk of the refactored code", + "tool": "detect_changes_tool", + "suggestion": "Check the impact of the applied refactor", }, { - "tool": "semantic_search_nodes", - "suggestion": "Find related symbols to also rename", + "tool": "get_affected_flows_tool", + "suggestion": "See which execution flows may have changed", + }, + { + "tool": "query_graph_tool", + "suggestion": "Inspect remaining call sites or rename fallout", }, ], - "semantic_search_nodes": [ + "semantic_search_nodes_tool": [ { - "tool": "query_graph", + "tool": "query_graph_tool", "suggestion": "Inspect callers/callees of a search result", }, { - "tool": "get_flow", + "tool": "get_flow_tool", "suggestion": "See the execution flow through a matched node", }, { - "tool": "get_impact_radius", + "tool": "get_impact_radius_tool", "suggestion": "Check the blast radius from matched nodes", }, ], diff --git a/code_review_graph/prompts.py b/code_review_graph/prompts.py index 455a4e6b7..1ca9663ca 100644 --- a/code_review_graph/prompts.py +++ b/code_review_graph/prompts.py @@ -1,9 +1,9 @@ """MCP prompt templates for Code Review Graph. Provides 5 pre-built prompt workflows, all enforcing token-efficient -detail_level="minimal" first patterns with get_minimal_context entry point. +detail_level="minimal" first patterns with get_minimal_context_tool entry point. -1. review_changes - pre-commit review using detect_changes + affected_flows +1. review_changes - pre-commit review using detect_changes_tool + get_affected_flows_tool 2. architecture_map - architecture docs using communities, flows, Mermaid 3. debug_issue - guided debugging using search, flow tracing 4. onboard_developer - new dev orientation using stats, architecture, flows @@ -17,15 +17,15 @@ _TOKEN_EFFICIENCY_PREAMBLE = ( # nosec B105 — prompt template, not a password """\ ## Rules for Token-Efficient Graph Usage -1. ALWAYS call `get_minimal_context` first with a task description. +1. ALWAYS call `get_minimal_context_tool` first with a task description. 2. Use `detail_level="minimal"` on all tool calls unless the minimal output \ is insufficient. 3. Only escalate to `detail_level="standard"` or `"verbose"` for the specific \ entities that need deeper inspection. 4. Never request more than 3 tool calls per turn unless absolutely necessary. -5. Prefer targeted queries (query_graph with a specific symbol) over broad \ -scans (list_communities with full members). -6. When reviewing changes: detect_changes(detail_level="minimal") → only \ +5. Prefer targeted queries (query_graph_tool with a specific symbol) over broad \ +scans (list_communities_tool with full members). +6. When reviewing changes: detect_changes_tool(detail_level="minimal") → only \ expand on high-risk items. """ ) @@ -51,22 +51,22 @@ def review_changes_prompt(base: str = "HEAD~1") -> list[Message]: return _user( f"{_TOKEN_EFFICIENCY_PREAMBLE}\n" f"## Review Workflow\n" - f'1. Call `get_minimal_context(task="review changes against ' + f'1. Call `get_minimal_context_tool(task="review changes against ' f'{base}")` to get risk overview.\n' f'2. If risk is "low": call ' - f'`detect_changes(detail_level="minimal")` → report summary ' + f'`detect_changes_tool(detail_level="minimal")` → report summary ' f"+ any test gaps.\n" f'3. If risk is "medium" or "high":\n' - f' a. Call `detect_changes(detail_level="standard")` for ' + f' a. Call `detect_changes_tool(detail_level="standard")` for ' f"full change list.\n" f" b. For each high-risk function, call " - f'`query_graph(pattern="callers_of", target=, ' + f'`query_graph_tool(pattern="callers_of", target=, ' f'detail_level="minimal")`.\n' - f' c. Call `get_affected_flows(detail_level="minimal")` ' + f' c. Call `get_affected_flows_tool(detail_level="minimal")` ' f"only if >3 changed functions.\n" f"4. Summarize: risk level, what changed, test gaps, " f"specific improvements needed.\n\n" - f"Do NOT call get_review_context unless you need source code " + f"Do NOT call get_review_context_tool unless you need source code " f"snippets for a specific function." ) @@ -76,12 +76,12 @@ def architecture_map_prompt() -> list[Message]: return _user( f"{_TOKEN_EFFICIENCY_PREAMBLE}\n" "## Architecture Mapping Workflow\n" - '1. Call `get_minimal_context(task="map architecture")`.\n' - '2. Call `get_architecture_overview(detail_level="minimal")` ' + '1. Call `get_minimal_context_tool(task="map architecture")`.\n' + '2. Call `get_architecture_overview_tool(detail_level="minimal")` ' "for community coupling summary.\n" - '3. Call `list_flows(detail_level="minimal")` for critical ' + '3. Call `list_flows_tool(detail_level="minimal")` for critical ' "flow names + criticality scores.\n" - "4. Only call `get_community(name=, " + "4. Only call `get_community_tool(name=, " 'detail_level="standard")` for the 1-2 communities the user ' "is most interested in.\n" "5. Produce a concise Mermaid diagram showing communities as " @@ -99,17 +99,17 @@ def debug_issue_prompt(description: str = "") -> list[Message]: return _user( f"{_TOKEN_EFFICIENCY_PREAMBLE}\n" "## Debug Workflow\n" - f'1. Call `get_minimal_context(task="debug: ' + f'1. Call `get_minimal_context_tool(task="debug: ' f'{desc_part}")`.\n' - "2. Call `semantic_search_nodes(query=, detail_level="minimal", limit=5)`.\n' "3. For the top 1-2 results, call " - '`query_graph(pattern="callers_of", target=, ' + '`query_graph_tool(pattern="callers_of", target=, ' 'detail_level="minimal")`.\n' "4. If the issue involves execution flow: call " - "`get_flow(name=)` for the single most " + "`get_flow_tool(name=)` for the single most " "relevant flow.\n" - "5. Only call `get_review_context` or `get_impact_radius` " + "5. Only call `get_review_context_tool` or `get_impact_radius_tool` " "if you need to trace the blast radius of a specific change." ) @@ -119,13 +119,13 @@ def onboard_developer_prompt() -> list[Message]: return _user( f"{_TOKEN_EFFICIENCY_PREAMBLE}\n" "## Onboarding Workflow\n" - '1. Call `get_minimal_context(task="onboard developer")`.\n' - "2. Call `list_graph_stats()` for technology overview.\n" - '3. Call `get_architecture_overview(detail_level="minimal")` ' + '1. Call `get_minimal_context_tool(task="onboard developer")`.\n' + "2. Call `list_graph_stats_tool()` for technology overview.\n" + '3. Call `get_architecture_overview_tool(detail_level="minimal")` ' "for the 30-second mental model.\n" - '4. Call `list_communities(detail_level="minimal")` — ' + '4. Call `list_communities_tool(detail_level="minimal")` — ' "present as a table of module names + sizes.\n" - '5. Call `list_flows(detail_level="minimal")` — highlight ' + '5. Call `list_flows_tool(detail_level="minimal")` — highlight ' "the top 3 critical flows.\n" "6. Only drill into a specific community or flow if the " "developer asks." @@ -141,19 +141,22 @@ def pre_merge_check_prompt(base: str = "HEAD~1") -> list[Message]: return _user( f"{_TOKEN_EFFICIENCY_PREAMBLE}\n" "## Pre-Merge Check Workflow\n" - '1. Call `get_minimal_context(task="pre-merge check")`.\n' - '2. Call `detect_changes(detail_level="minimal")` for risk ' + '1. Call `get_minimal_context_tool(task="pre-merge check")`.\n' + '2. Call `detect_changes_tool(detail_level="minimal")` for risk ' "score and test gaps.\n" "3. If risk > 0.4: call " - '`get_affected_flows(detail_level="minimal")`.\n' + '`get_affected_flows_tool(detail_level="minimal")`.\n' "4. If test_gap_count > 0: call " - '`query_graph(pattern="tests_for", ' + '`query_graph_tool(pattern="tests_for", ' 'target=, detail_level="minimal")` ' "for up to 3 functions.\n" - '5. Call `refactor(mode="dead_code", ' + '5. Call `refactor_tool(mode="dead_code", ' 'detail_level="minimal")` to check for newly dead code.\n' - "6. Only call `find_large_functions` or `get_impact_radius` " + "6. If you are planning a rename, also call " + "`semantic_search_nodes_tool(detail_level=\"minimal\")` to find " + "related symbols to also rename.\n" + "7. Only call `find_large_functions_tool` or `get_impact_radius_tool` " "if risk > 0.7.\n" - "7. Output: GO/NO-GO recommendation with 1-sentence " + "8. Output: GO/NO-GO recommendation with 1-sentence " "justification + list of required follow-ups." ) diff --git a/code_review_graph/skills.py b/code_review_graph/skills.py index 8088e127b..905906d01 100644 --- a/code_review_graph/skills.py +++ b/code_review_graph/skills.py @@ -680,20 +680,20 @@ def _record_configured(key: str, plat: dict[str, Any]) -> None: "## Explore Codebase\n\n" "Use the code-review-graph MCP tools to explore and understand the codebase.\n\n" "### Steps\n\n" - "1. Run `list_graph_stats` to see overall codebase metrics.\n" + "1. Run `list_graph_stats_tool` to see overall codebase metrics.\n" "2. Run `get_architecture_overview_tool` for high-level community structure.\n" - "3. Use `list_communities_tool` to find major modules, then `get_community` " + "3. Use `list_communities_tool` to find major modules, then `get_community_tool` " "for details.\n" "4. Use `semantic_search_nodes_tool` to find specific functions or classes.\n" "5. Use `query_graph_tool` with patterns like `callers_of`, `callees_of`, " "`imports_of` to trace relationships.\n" - "6. Use `list_flows` and `get_flow` to understand execution paths.\n\n" + "6. Use `list_flows_tool` and `get_flow_tool` to understand execution paths.\n\n" "### Tips\n\n" "- Start broad (stats, architecture) then narrow down to specific areas.\n" "- Use `children_of` on a file to see all its functions and classes.\n" - "- Use `find_large_functions` to identify complex code.\n\n" + "- Use `find_large_functions_tool` to identify complex code.\n\n" "## Token Efficiency Rules\n" - '- ALWAYS start with `get_minimal_context(task="")` ' + '- ALWAYS start with `get_minimal_context_tool(task="")` ' "before any other graph tool.\n" '- Use `detail_level="minimal"` on all calls. Only escalate to ' '"standard" when minimal is insufficient.\n' @@ -721,7 +721,7 @@ def _record_configured(key: str, plat: dict[str, Any]) -> None: "- Suggested improvements\n" "- Overall merge recommendation\n\n" "## Token Efficiency Rules\n" - '- ALWAYS start with `get_minimal_context(task="")` ' + '- ALWAYS start with `get_minimal_context_tool(task="")` ' "before any other graph tool.\n" '- Use `detail_level="minimal"` on all calls. Only escalate to ' '"standard" when minimal is insufficient.\n' @@ -739,7 +739,7 @@ def _record_configured(key: str, plat: dict[str, Any]) -> None: "1. Use `semantic_search_nodes_tool` to find code related to the issue.\n" "2. Use `query_graph_tool` with `callers_of` and `callees_of` to trace " "call chains.\n" - "3. Use `get_flow` to see full execution paths through suspected areas.\n" + "3. Use `get_flow_tool` to see full execution paths through suspected areas.\n" "4. Run `detect_changes_tool` to check if recent changes caused the issue.\n" "5. Use `get_impact_radius_tool` on suspected files to see what else is affected.\n\n" "### Tips\n\n" @@ -747,7 +747,7 @@ def _record_configured(key: str, plat: dict[str, Any]) -> None: "- Look at affected flows to find the entry point that triggers the bug.\n" "- Recent changes are the most common source of new issues.\n\n" "## Token Efficiency Rules\n" - '- ALWAYS start with `get_minimal_context(task="")` ' + '- ALWAYS start with `get_minimal_context_tool(task="")` ' "before any other graph tool.\n" '- Use `detail_level="minimal"` on all calls. Only escalate to ' '"standard" when minimal is insufficient.\n' @@ -773,9 +773,9 @@ def _record_configured(key: str, plat: dict[str, Any]) -> None: "- Always preview before applying (rename mode gives you an edit list).\n" "- Check `get_impact_radius_tool` before major refactors.\n" "- Use `get_affected_flows_tool` to ensure no critical paths are broken.\n" - "- Run `find_large_functions` to identify decomposition targets.\n\n" + "- Run `find_large_functions_tool` to identify decomposition targets.\n\n" "## Token Efficiency Rules\n" - '- ALWAYS start with `get_minimal_context(task="")` ' + '- ALWAYS start with `get_minimal_context_tool(task="")` ' "before any other graph tool.\n" '- Use `detail_level="minimal"` on all calls. Only escalate to ' '"standard" when minimal is insufficient.\n' diff --git a/code_review_graph/tools/community_tools.py b/code_review_graph/tools/community_tools.py index 4c6647c65..eb2334e9f 100644 --- a/code_review_graph/tools/community_tools.py +++ b/code_review_graph/tools/community_tools.py @@ -56,7 +56,7 @@ def list_communities_func( "communities": communities, } result["_hints"] = generate_hints( - "list_communities", result, get_session() + "list_communities_tool", result, get_session() ) return result except Exception as exc: @@ -132,7 +132,7 @@ def get_community_func( "community": community, } result["_hints"] = generate_hints( - "get_community", result, get_session() + "get_community_tool", result, get_session() ) return result except Exception as exc: @@ -235,7 +235,7 @@ def get_architecture_overview_func( **overview, } result["_hints"] = generate_hints( - "get_architecture_overview", result, get_session() + "get_architecture_overview_tool", result, get_session() ) if detail_level == "minimal": attach_context_savings(result, original_context=full_overview) diff --git a/code_review_graph/tools/flows_tools.py b/code_review_graph/tools/flows_tools.py index 4df1f713c..073ca7c8b 100644 --- a/code_review_graph/tools/flows_tools.py +++ b/code_review_graph/tools/flows_tools.py @@ -73,7 +73,7 @@ def list_flows( "flows": flows, } result["_hints"] = generate_hints( - "list_flows", result, get_session() + "list_flows_tool", result, get_session() ) return result except Exception as exc: @@ -167,7 +167,7 @@ def get_flow( "flow": flow, } result["_hints"] = generate_hints( - "get_flow", result, get_session() + "get_flow_tool", result, get_session() ) return result except Exception as exc: diff --git a/code_review_graph/tools/query.py b/code_review_graph/tools/query.py index 5fdbd1e36..fb31735db 100644 --- a/code_review_graph/tools/query.py +++ b/code_review_graph/tools/query.py @@ -765,7 +765,7 @@ def semantic_search_nodes( "results": results, } result["_hints"] = generate_hints( - "semantic_search_nodes", result, get_session() + "semantic_search_nodes_tool", result, get_session() ) return result finally: diff --git a/code_review_graph/tools/refactor_tools.py b/code_review_graph/tools/refactor_tools.py index c92891b3f..fe03efeff 100644 --- a/code_review_graph/tools/refactor_tools.py +++ b/code_review_graph/tools/refactor_tools.py @@ -84,7 +84,7 @@ def refactor_func( **preview, } result["_hints"] = generate_hints( - "refactor", result, get_session() + "refactor_tool", result, get_session() ) return result @@ -99,7 +99,7 @@ def refactor_func( "total": len(dead), } result["_hints"] = generate_hints( - "refactor", result, get_session() + "refactor_tool", result, get_session() ) return result @@ -115,7 +115,7 @@ def refactor_func( "total": len(suggestions), } result["_hints"] = generate_hints( - "refactor", result, get_session() + "refactor_tool", result, get_session() ) return result @@ -165,4 +165,7 @@ def apply_refactor_func( return {"status": "error", "error": str(exc)} result = apply_refactor(refactor_id, root, dry_run=dry_run) + result["_hints"] = generate_hints( + "apply_refactor_tool", result, get_session() + ) return result diff --git a/code_review_graph/tools/review.py b/code_review_graph/tools/review.py index 9b8775922..44de38b0e 100644 --- a/code_review_graph/tools/review.py +++ b/code_review_graph/tools/review.py @@ -114,8 +114,8 @@ def get_review_context( "key_entities": key_entities, "test_gaps": test_gap_count, "next_tool_suggestions": [ - "detect_changes", - "get_affected_flows", + "detect_changes_tool", + "get_affected_flows_tool", "get_impact_radius", ], } @@ -341,7 +341,7 @@ def get_affected_flows_func( "total": total, } out["_hints"] = generate_hints( - "get_affected_flows", out, get_session() + "get_affected_flows_tool", out, get_session() ) return out except Exception as exc: @@ -470,7 +470,7 @@ def detect_changes_func( **analysis, } result["_hints"] = generate_hints( - "detect_changes", result, get_session() + "detect_changes_tool", result, get_session() ) attach_context_savings(result, original_tokens=original_tokens) return result diff --git a/skills/debug-issue/SKILL.md b/skills/debug-issue/SKILL.md index b8d928fa3..a0fc76935 100644 --- a/skills/debug-issue/SKILL.md +++ b/skills/debug-issue/SKILL.md @@ -11,7 +11,7 @@ Use the knowledge graph to systematically trace and debug issues. 1. Use `semantic_search_nodes_tool` to find code related to the issue. 2. Use `query_graph_tool` with `callers_of` and `callees_of` to trace call chains. -3. Use `get_flow` to see full execution paths through suspected areas. +3. Use `get_flow_tool` to see full execution paths through suspected areas. 4. Run `detect_changes_tool` to check if recent changes caused the issue. 5. Use `get_impact_radius_tool` on suspected files to see what else is affected. @@ -22,6 +22,6 @@ Use the knowledge graph to systematically trace and debug issues. - Recent changes are the most common source of new issues. ## Token Efficiency Rules -- ALWAYS start with `get_minimal_context(task="")` before any other graph tool. +- ALWAYS start with `get_minimal_context_tool(task="")` before any other graph tool. - Use `detail_level="minimal"` on all calls. Only escalate to "standard" when minimal is insufficient. - Target: complete any review/debug/refactor task in ≤5 tool calls and ≤800 total output tokens. diff --git a/skills/explore-codebase/SKILL.md b/skills/explore-codebase/SKILL.md index a470b3c8d..74dec19b2 100644 --- a/skills/explore-codebase/SKILL.md +++ b/skills/explore-codebase/SKILL.md @@ -9,20 +9,20 @@ Use the code-review-graph MCP tools to explore and understand the codebase. ### Steps -1. Run `list_graph_stats` to see overall codebase metrics. +1. Run `list_graph_stats_tool` to see overall codebase metrics. 2. Run `get_architecture_overview_tool` for high-level community structure. -3. Use `list_communities_tool` to find major modules, then `get_community` for details. +3. Use `list_communities_tool` to find major modules, then `get_community_tool` for details. 4. Use `semantic_search_nodes_tool` to find specific functions or classes. 5. Use `query_graph_tool` with patterns like `callers_of`, `callees_of`, `imports_of` to trace relationships. -6. Use `list_flows` and `get_flow` to understand execution paths. +6. Use `list_flows_tool` and `get_flow_tool` to understand execution paths. ### Tips - Start broad (stats, architecture) then narrow down to specific areas. - Use `children_of` on a file to see all its functions and classes. -- Use `find_large_functions` to identify complex code. +- Use `find_large_functions_tool` to identify complex code. ## Token Efficiency Rules -- ALWAYS start with `get_minimal_context(task="")` before any other graph tool. +- ALWAYS start with `get_minimal_context_tool(task="")` before any other graph tool. - Use `detail_level="minimal"` on all calls. Only escalate to "standard" when minimal is insufficient. - Target: complete any review/debug/refactor task in ≤5 tool calls and ≤800 total output tokens. diff --git a/skills/refactor-safely/SKILL.md b/skills/refactor-safely/SKILL.md index 3020efb5c..410b5c397 100644 --- a/skills/refactor-safely/SKILL.md +++ b/skills/refactor-safely/SKILL.md @@ -20,9 +20,9 @@ Use the knowledge graph to plan and execute refactoring with confidence. - Always preview before applying (rename mode gives you an edit list). - Check `get_impact_radius_tool` before major refactors. - Use `get_affected_flows_tool` to ensure no critical paths are broken. -- Run `find_large_functions` to identify decomposition targets. +- Run `find_large_functions_tool` to identify decomposition targets. ## Token Efficiency Rules -- ALWAYS start with `get_minimal_context(task="")` before any other graph tool. +- ALWAYS start with `get_minimal_context_tool(task="")` before any other graph tool. - Use `detail_level="minimal"` on all calls. Only escalate to "standard" when minimal is insufficient. - Target: complete any review/debug/refactor task in ≤5 tool calls and ≤800 total output tokens. diff --git a/skills/review-changes/SKILL.md b/skills/review-changes/SKILL.md index a5c5e68cd..c62810d2b 100644 --- a/skills/review-changes/SKILL.md +++ b/skills/review-changes/SKILL.md @@ -24,6 +24,6 @@ Provide findings grouped by risk level (high/medium/low) with: - Overall merge recommendation ## Token Efficiency Rules -- ALWAYS start with `get_minimal_context(task="")` before any other graph tool. +- ALWAYS start with `get_minimal_context_tool(task="")` before any other graph tool. - Use `detail_level="minimal"` on all calls. Only escalate to "standard" when minimal is insufficient. - Target: complete any review/debug/refactor task in ≤5 tool calls and ≤800 total output tokens. diff --git a/tests/test_hints.py b/tests/test_hints.py index 14d9967d3..f4ac82f19 100644 --- a/tests/test_hints.py +++ b/tests/test_hints.py @@ -19,24 +19,30 @@ def test_fresh_session_exploring(self): def test_review_intent_detected(self): """Recording review-oriented tools should infer 'reviewing'.""" session = SessionState() - for tool in ("detect_changes", "get_review_context", "get_affected_flows"): + for tool in ("detect_changes_tool", "get_review_context_tool", "get_affected_flows_tool"): session.record_tool_call(tool) assert infer_intent(session) == "reviewing" def test_debug_intent_detected(self): """Recording debug-oriented tools should infer 'debugging'.""" session = SessionState() - for tool in ("query_graph", "get_flow", "semantic_search_nodes"): + for tool in ("query_graph_tool", "get_flow_tool", "semantic_search_nodes_tool"): session.record_tool_call(tool) assert infer_intent(session) == "debugging" def test_refactoring_intent_detected(self): """Recording refactoring-oriented tools should infer 'refactoring'.""" session = SessionState() - for tool in ("refactor", "find_dead_code", "suggest_refactorings"): + for tool in ("refactor_tool", "apply_refactor_tool"): session.record_tool_call(tool) assert infer_intent(session) == "refactoring" + def test_large_functions_tool_does_not_force_refactoring_intent(self): + """find_large_functions_tool is a size query, not a refactor action.""" + session = SessionState() + session.record_tool_call("find_large_functions_tool") + assert infer_intent(session) == "exploring" + def test_session_caps_history(self): """tools_called should never exceed 100 entries (FIFO).""" session = SessionState() @@ -58,28 +64,28 @@ class TestGenerateHints: def test_hints_no_repeat(self): """Already-called tools must not appear in next_steps.""" session = SessionState() - # Call list_flows, then generate hints for it - # list_flows suggests get_flow, get_affected_flows, get_architecture_overview - generate_hints("list_flows", {"status": "ok"}, session) + # Call list_flows_tool, then generate hints for it + # list_flows_tool suggests get_flow_tool, get_affected_flows_tool, get_architecture_overview_tool + generate_hints("list_flows_tool", {"status": "ok"}, session) - # Now call get_flow and regenerate hints for list_flows - hints2 = generate_hints("list_flows", {"status": "ok"}, session) + # Now call get_flow_tool and regenerate hints for list_flows_tool + hints2 = generate_hints("list_flows_tool", {"status": "ok"}, session) suggested_tools2 = {s["tool"] for s in hints2["next_steps"]} - # list_flows itself was called, so it shouldn't be suggested by get_flow workflow - # Also, the first list_flows call should be excluded from next suggestions - assert "list_flows" not in suggested_tools2 + # list_flows_tool itself was called, so it should not be suggested by the get_flow_tool workflow + # Also, the first list_flows_tool call should be excluded from next suggestions + assert "list_flows_tool" not in suggested_tools2 def test_hints_max_three(self): """Each hints category should have at most 3 entries.""" session = SessionState() - # detect_changes has 4 workflow entries + # detect_changes_tool has 4 workflow entries result = { "status": "ok", "test_gaps": [{"name": f"gap_{i}"} for i in range(10)], "risk_score": 0.9, "warnings": ["coupling warning 1", "coupling warning 2"], } - hints = generate_hints("detect_changes", result, session) + hints = generate_hints("detect_changes_tool", result, session) assert len(hints["next_steps"]) <= _MAX_PER_CATEGORY assert len(hints["warnings"]) <= _MAX_PER_CATEGORY assert len(hints["related"]) <= _MAX_PER_CATEGORY @@ -91,7 +97,7 @@ def test_warnings_from_result_test_gaps(self): "status": "ok", "test_gaps": [{"name": "untested_func"}, {"name": "another_func"}], } - hints = generate_hints("detect_changes", result, session) + hints = generate_hints("detect_changes_tool", result, session) assert any("Test coverage gaps" in w for w in hints["warnings"]) assert any("untested_func" in w for w in hints["warnings"]) @@ -99,20 +105,20 @@ def test_warnings_from_result_risk_score(self): """High risk_score in result should produce a warning.""" session = SessionState() result = {"status": "ok", "risk_score": 0.85} - hints = generate_hints("detect_changes", result, session) + hints = generate_hints("detect_changes_tool", result, session) assert any("High risk score" in w for w in hints["warnings"]) def test_warnings_low_risk_no_warning(self): """Low risk_score should NOT produce a warning.""" session = SessionState() result = {"status": "ok", "risk_score": 0.3} - hints = generate_hints("detect_changes", result, session) + hints = generate_hints("detect_changes_tool", result, session) assert not any("High risk score" in w for w in hints["warnings"]) def test_generate_hints_empty_result(self): """An empty/minimal result should still return valid hints structure.""" session = SessionState() - hints = generate_hints("list_flows", {}, session) + hints = generate_hints("list_flows_tool", {}, session) assert "next_steps" in hints assert "related" in hints assert "warnings" in hints @@ -131,7 +137,7 @@ def test_session_records_files(self): """Files from result should be tracked in session state.""" session = SessionState() result = {"status": "ok", "changed_files": ["a.py", "b.py"]} - generate_hints("detect_changes", result, session) + generate_hints("detect_changes_tool", result, session) assert "a.py" in session.files_touched assert "b.py" in session.files_touched @@ -145,7 +151,7 @@ def test_session_records_nodes(self): {"qualified_name": "mod.py::Bar", "name": "Bar"}, ], } - generate_hints("semantic_search_nodes", result, session) + generate_hints("semantic_search_nodes_tool", result, session) assert "mod.py::Foo" in session.nodes_queried assert "mod.py::Bar" in session.nodes_queried @@ -157,10 +163,35 @@ def test_related_suggests_untouched_files(self): "status": "ok", "impacted_files": ["already_seen.py", "new_file.py", "other.py"], } - hints = generate_hints("detect_changes", result, session) + hints = generate_hints("detect_changes_tool", result, session) assert "already_seen.py" not in hints["related"] assert "new_file.py" in hints["related"] + def test_apply_refactor_hints(self): + """apply_refactor_tool should surface follow-up review actions.""" + session = SessionState() + result = { + "status": "ok", + "applied": True, + "changed_files": ["a.py"], + } + hints = generate_hints("apply_refactor_tool", result, session) + suggested_tools = {step["tool"] for step in hints["next_steps"]} + assert "detect_changes_tool" in suggested_tools + assert "get_affected_flows_tool" in suggested_tools + + def test_refactor_hints_include_related_symbol_search(self): + """refactor_tool should suggest searching related symbols to rename.""" + session = SessionState() + result = { + "status": "ok", + "previewed": True, + "changes": [], + } + hints = generate_hints("refactor_tool", result, session) + suggested_tools = {step["tool"] for step in hints["next_steps"]} + assert "semantic_search_nodes_tool" in suggested_tools + class TestGlobalSession: def test_get_session_returns_singleton(self): @@ -190,6 +221,6 @@ def test_warnings_from_arch_overview_dict(self): {"message": "Circular dependency detected"}, ], } - hints = generate_hints("get_architecture_overview", result, session) + hints = generate_hints("get_architecture_overview_tool", result, session) assert any("High coupling" in w for w in hints["warnings"]) assert any("Circular dependency" in w for w in hints["warnings"]) diff --git a/tests/test_prompts.py b/tests/test_prompts.py index 1714342f6..fa6a069e4 100644 --- a/tests/test_prompts.py +++ b/tests/test_prompts.py @@ -39,11 +39,11 @@ def test_custom_base(self): def test_mentions_detect_changes(self): result = review_changes_prompt() - assert "detect_changes" in _text(result[0]) + assert "detect_changes_tool" in _text(result[0]) def test_mentions_affected_flows(self): result = review_changes_prompt() - assert "affected_flows" in _text(result[0]) + assert "get_affected_flows_tool" in _text(result[0]) def test_mentions_test_gaps(self): result = review_changes_prompt() @@ -96,11 +96,11 @@ def test_empty_description(self): def test_mentions_search(self): result = debug_issue_prompt(description="test issue") - assert "semantic_search_nodes" in _text(result[0]) + assert "semantic_search_nodes_tool" in _text(result[0]) def test_mentions_get_minimal_context(self): result = debug_issue_prompt() - assert "get_minimal_context" in _text(result[0]) + assert "get_minimal_context_tool" in _text(result[0]) class TestOnboardDeveloperPrompt: @@ -118,7 +118,7 @@ def test_message_has_role_and_content(self): def test_mentions_stats(self): result = onboard_developer_prompt() - assert "list_graph_stats" in _text(result[0]) + assert "list_graph_stats_tool" in _text(result[0]) def test_mentions_architecture(self): result = onboard_developer_prompt() @@ -158,13 +158,21 @@ def test_mentions_risk_scoring(self): result = pre_merge_check_prompt() assert "risk" in _text(result[0]).lower() + def test_mentions_related_symbol_search(self): + result = pre_merge_check_prompt() + assert "semantic_search_nodes_tool" in _text(result[0]) + def test_mentions_test_gaps(self): result = pre_merge_check_prompt() - assert "tests_for" in _text(result[0]) + assert "query_graph_tool" in _text(result[0]) + + def test_mentions_dead_code_mode(self): + result = pre_merge_check_prompt() + assert 'refactor_tool(mode="dead_code"' in _text(result[0]) - def test_mentions_dead_code(self): + def test_mentions_large_functions_tool(self): result = pre_merge_check_prompt() - assert "dead_code" in _text(result[0]) + assert "find_large_functions_tool" in _text(result[0]) class TestTokenEfficiencyPreamble: @@ -172,21 +180,21 @@ class TestTokenEfficiencyPreamble: def test_review_has_preamble(self): result = review_changes_prompt() - assert "get_minimal_context" in _text(result[0]) + assert "get_minimal_context_tool" in _text(result[0]) assert "detail_level" in _text(result[0]) def test_architecture_has_preamble(self): result = architecture_map_prompt() - assert "get_minimal_context" in _text(result[0]) + assert "get_minimal_context_tool" in _text(result[0]) def test_debug_has_preamble(self): result = debug_issue_prompt() - assert "get_minimal_context" in _text(result[0]) + assert "get_minimal_context_tool" in _text(result[0]) def test_onboard_has_preamble(self): result = onboard_developer_prompt() - assert "get_minimal_context" in _text(result[0]) + assert "get_minimal_context_tool" in _text(result[0]) def test_pre_merge_has_preamble(self): result = pre_merge_check_prompt() - assert "get_minimal_context" in _text(result[0]) + assert "get_minimal_context_tool" in _text(result[0]) diff --git a/tests/test_skills.py b/tests/test_skills.py index dd54eca90..5aac357d7 100644 --- a/tests/test_skills.py +++ b/tests/test_skills.py @@ -170,14 +170,51 @@ def test_custom_skills_dir(self, tmp_path): assert len(list(result.iterdir())) == 4 def test_skill_content_includes_get_minimal_context(self, tmp_path): - """Every skill template must reference get_minimal_context.""" + """Every skill template must reference get_minimal_context_tool.""" skills_dir = generate_skills(tmp_path) for subdir in skills_dir.iterdir(): content = (subdir / "SKILL.md").read_text() - assert "get_minimal_context" in content, ( - f"{subdir.name} missing get_minimal_context reference" + assert "get_minimal_context_tool" in content, ( + f"{subdir.name} missing get_minimal_context_tool reference" ) + def test_skill_templates_use_exported_tool_names(self, tmp_path): + generated = generate_skills(tmp_path) + bundled = Path(__file__).parents[1] / "skills" + + expected_tools = { + "explore-codebase": [ + "get_minimal_context_tool", + "list_graph_stats_tool", + "get_community_tool", + "list_flows_tool", + "get_flow_tool", + "find_large_functions_tool", + ], + "review-changes": ["get_minimal_context_tool", "get_affected_flows_tool"], + "debug-issue": ["get_minimal_context_tool", "get_flow_tool"], + "refactor-safely": ["get_minimal_context_tool", "find_large_functions_tool"], + } + legacy_tools = [ + "get_minimal_context", + "list_graph_stats", + "get_community", + "list_flows", + "get_flow", + "find_large_functions", + ] + + for skill_name, tool_names in expected_tools.items(): + for skill_file in ( + generated / skill_name / "SKILL.md", + bundled / skill_name / "SKILL.md", + ): + content = skill_file.read_text(encoding="utf-8") + for tool_name in tool_names: + assert tool_name in content, skill_file + for legacy_tool in legacy_tools: + assert f"`{legacy_tool}`" not in content, skill_file + def test_skill_content_includes_detail_level(self, tmp_path): """Every skill template must reference detail_level.""" skills_dir = generate_skills(tmp_path)