diff --git a/docs/AGENT-GUIDE.md b/docs/AGENT-GUIDE.md index 2045ed5..8c0aec1 100644 --- a/docs/AGENT-GUIDE.md +++ b/docs/AGENT-GUIDE.md @@ -139,21 +139,32 @@ Use `search` to recover the stored symbol id / FQN if you only have a simple nam Omitting them is a validation error. This is intentional: it prevents huge accidental fan-out. -Optional `filter` applies to the **other** endpoint node (same `NodeFilter` keys as `find`; keys irrelevant to that node kind are ignored). +Optional `filter` applies to the **other** endpoint node using the same `NodeFilter` schema as `find`. Populated fields must be applicable to that neighbor's kind; mixed-kind neighborhoods fail loud on the first neighbor row whose kind rejects the filter. #### E. Shared `NodeFilter` (for `find`, `search.filter`, `neighbors.filter`) -One object shape everywhere. **For `find`, `filter` is required** — use at least one key (e.g. `{"microservice":"chat-core"}`) or `{}` is valid Pydantic but may be expensive at scale; prefer narrowing keys. +One object shape everywhere. **For `find`, `filter` is required** — `{}` is valid (no predicates; returns the full kind up to pagination) but may be expensive at scale; prefer narrowing keys when you can. | Keys | Applies to | | ---- | ---------- | | `microservice`, `module`, `source_layer` | All kinds (`source_layer` mainly **client**: `builtin` / brownfield) | -| `role`, `exclude_roles`, `annotation`, `capability`, `fqn_prefix`, `symbol_kind`, `symbol_kinds` | **symbol** (ignored for route/client) | +| `role`, `exclude_roles`, `annotation`, `capability`, `fqn_prefix`, `symbol_kind`, `symbol_kinds` | **symbol** | | `http_method`, `path_prefix`, `framework` | **route** | | `client_kind`, `target_service`, `target_path_prefix`, `http_method` | **client** | The same `http_method` key filters HTTP verbs on **routes** (server-side declared method) and on **clients** (caller-side method on the outbound call). It is not applicable to **symbol** rows. +### Strict frame contract (`find`, `search.filter`, `neighbors.filter`) + +- **One populated field, one stored attribute** for the evaluated kind. Inapplicable fields or `extra` keys are never silently dropped: the tool returns `success=false` with a teaching message (and applicable-field list for cross-kind mistakes). +- **No wildcards** in `fqn_prefix`, `path_prefix`, or `target_path_prefix` (`*` / `?` rejected). Use `search(query=…)` for ranked text discovery instead. +- **`search.query` is not a DSL** — treat it as opaque text scored against the index. Structured predicates belong in `find`. +- **`neighbors` filters neighbor rows by kind** — the first neighbor whose kind rejects the filter fails the whole call (no per-row silent skip). + +### Identifier resolution (pre-`resolve`) + +For identifier-shaped lookups without a stable graph id or exact symbol FQN, use **`search(query=…)`** for ranked candidates, then **`describe(id=…)`** (or `describe(fqn=…)` when you have an exact FQN) on each promising row until you confirm the right node. A dedicated **`resolve`** tool is planned separately; until it ships, this multi-call pattern is the supported fallback. + **`source_layer` vs `role`:** On **Client** nodes, `source_layer` records which brownfield or built-in layer produced the client declaration (`builtin`, `layer_a_meta`, `layer_b_ann`, `layer_c_source`, `layer_b_fqn`, …). On **Symbol** nodes, `role` is the inferred architectural stereotype (`CONTROLLER`, `SERVICE`, `REPO`, …). They answer different questions; names stay distinct. **`target_service` vs `microservice`:** `microservice` is the service **where the node lives** (home service / owning module). `target_service` (clients only) is the **remote service being called**. A client in `operator-api` may list `partner-api` as `target_service`. @@ -190,7 +201,8 @@ Exact allowed values for roles, capabilities, client kinds, etc. live in `java_o #### `search` - **Purpose:** Locate chunk hits by NL or code fragment; use `symbol_id` when present to jump into the graph. -- **Args:** `query`, `table` (`java`|`sql`|`yaml`|`all`, default `java`), `hybrid` (bool), `limit` (default 5), `offset`, `path_contains`, optional `filter` (`NodeFilter` — post-filters hits using symbol-oriented fields on the row). +- **Args:** `query`, `table` (`java`|`sql`|`yaml`|`all`, default `java`), `hybrid` (bool), `limit` (default 5), `offset`, `path_contains`, optional `filter` (`NodeFilter` — post-filters hits using **symbol-applicable** fields only). +- **Strict frame:** `query` is opaque ranked text (no structured DSL inside the string). Optional `filter` follows the same strict applicability and wildcard rules as `find` for symbols. - **Tip:** For behaviour questions, narrow noise with `filter.exclude_roles` or `filter.role` when you know the shape you want. #### `find` @@ -202,7 +214,7 @@ Exact allowed values for roles, capabilities, client kinds, etc. live in `java_o #### `describe` - **Purpose:** Full node payload + `edge_summary`: `in` / `out` counts **per stored graph edge label** (what exists as edges in Kuzu). For **type** Symbols only (`class`, `interface`, `enum`, `record`, `annotation`), the same map may also include **describe-time composed** dot-keys — summaries of member edges, not stored labels — see the next bullets (`DECLARES.DECLARES_CLIENT`, `DECLARES.EXPOSES`); those keys are **not** valid in `neighbors(edge_types=…)`. For **method** Symbols, the map may include **override-axis** virtual keys (`OVERRIDDEN_BY`, `OVERRIDDEN_BY.DECLARES_CLIENT`, `OVERRIDDEN_BY.EXPOSES`, `OVERRIDES`); see **Override-axis keys (method Symbols)** below — also not `EdgeType` literals. -- **Args:** `id` (symbol, route, or client id). +- **Args:** `id` (symbol, route, or client id) or **`fqn`** (exact symbol FQN when you do not have the graph id). When both are set, `id` wins. For ambiguous identifiers without an exact id/FQN, see **Identifier resolution (pre-`resolve`)** above. **Composed `edge_summary` keys (type Symbols).** Keys use dot notation: `.`. Two are emitted today: diff --git a/mcp_v2.py b/mcp_v2.py index e4d00ca..f353f8f 100644 --- a/mcp_v2.py +++ b/mcp_v2.py @@ -8,7 +8,7 @@ prefix fields (``fqn_prefix``, ``path_prefix``, ``target_path_prefix``) reject ``*`` and ``?`` — see ``_validate_no_wildcards``. -Revisit trigger (``propose/MCP-FILTER-FRAME-PROPOSE.md`` section 3.4.6) +Revisit trigger (``propose/completed/MCP-FILTER-FRAME-PROPOSE.md`` section 3.4.6) -------------------------------------------------------------- If **three** legitimate issue-tracker workflows appear within **six months** of frame lock where the strict frame has no clean analog under ``search``, deferred @@ -19,6 +19,7 @@ import json import os +import sys from pathlib import Path import threading from typing import Annotated, Any, Literal @@ -60,6 +61,23 @@ _METHOD_SYMBOL_KINDS_FOR_OVERRIDE_ROLLUP = frozenset({"method"}) +_fail_loud_counts: dict[str, int] = {} +_fail_loud_lock = threading.Lock() + + +def _log_fail_loud(category: str) -> None: + """Increment process-local fail-loud counter and emit one stderr line (PR-FRAME-3).""" + with _fail_loud_lock: + _fail_loud_counts[category] = _fail_loud_counts.get(category, 0) + 1 + n = _fail_loud_counts[category] + print(f"[filter-frame] fail-loud category={category} count={n}", file=sys.stderr, flush=True) + + +def filter_frame_counters() -> dict[str, int]: + """Snapshot of fail-loud counts (tests / local diagnostics; not an MCP tool).""" + with _fail_loud_lock: + return dict(_fail_loud_counts) + def _get_sentence_transformer(model_name: str, device: str | None) -> SentenceTransformer: global _st_model @@ -529,10 +547,13 @@ def search_v2( else raw_filter ) except ValidationError as exc: + _log_fail_loud("unknown_key") return SearchOutput(success=False, message=_filter_validation_error_message(exc)) if nf and (err := _nodefilter_applicability_error("symbol", nf)): + _log_fail_loud("applicability") return SearchOutput(success=False, message=err) if nf and (err := _validate_no_wildcards(nf)): + _log_fail_loud("wildcard") return SearchOutput(success=False, message=err) model_name = resolved_sbert_model_for_process_env(SBERT_MODEL) device = os.environ.get("SBERT_DEVICE") or None @@ -585,10 +606,13 @@ def find_v2( try: nf = NodeFilter.model_validate(raw_filter) if not isinstance(raw_filter, NodeFilter) else raw_filter except ValidationError as exc: + _log_fail_loud("unknown_key") return FindOutput(success=False, message=_filter_validation_error_message(exc)) if err := _nodefilter_applicability_error(kind, nf): + _log_fail_loud("applicability") return FindOutput(success=False, message=err) if err := _validate_no_wildcards(nf): + _log_fail_loud("wildcard") return FindOutput(success=False, message=err) if kind == "symbol": where, params = _symbol_where_from_filter(nf) @@ -700,8 +724,10 @@ def neighbors_v2( else raw_filter ) except ValidationError as exc: + _log_fail_loud("unknown_key") return NeighborsOutput(success=False, message=_filter_validation_error_message(exc)) if nf and (err := _validate_no_wildcards(nf)): + _log_fail_loud("wildcard") return NeighborsOutput(success=False, message=err) origins = [ids] if isinstance(ids, str) else list(ids) results: list[Edge] = [] @@ -739,6 +765,7 @@ def neighbors_v2( if other_rec is None: continue if nf and (err := _nodefilter_applicability_error(other_kind, nf)): + _log_fail_loud("applicability") return NeighborsOutput(success=False, message=err) if not _node_matches_filter(other_kind, other_rec, nf): continue diff --git a/plans/CURSOR-PROMPTS-MCP-FILTER-FRAME.md b/plans/completed/CURSOR-PROMPTS-MCP-FILTER-FRAME.md similarity index 98% rename from plans/CURSOR-PROMPTS-MCP-FILTER-FRAME.md rename to plans/completed/CURSOR-PROMPTS-MCP-FILTER-FRAME.md index 4a80821..d25668f 100644 --- a/plans/CURSOR-PROMPTS-MCP-FILTER-FRAME.md +++ b/plans/completed/CURSOR-PROMPTS-MCP-FILTER-FRAME.md @@ -1,6 +1,9 @@ # Cursor task prompts — MCP Filter Frame (PR-FRAME-1 → PR-FRAME-3) -Status: **active**. One prompt per PR; each prompt is self-contained. +Status: **completed** — reference template for the landed PR-FRAME-1 → PR-FRAME-3 +sequence. Plan: +[`PLAN-MCP-FILTER-FRAME.md`](PLAN-MCP-FILTER-FRAME.md); propose: +[`propose/completed/MCP-FILTER-FRAME-PROPOSE.md`](../../propose/completed/MCP-FILTER-FRAME-PROPOSE.md). One prompt per PR. Each is **self-contained**: copy the prompt verbatim into Cursor, attach the files listed in its `@-files` block, and let diff --git a/plans/PLAN-MCP-FILTER-FRAME.md b/plans/completed/PLAN-MCP-FILTER-FRAME.md similarity index 98% rename from plans/PLAN-MCP-FILTER-FRAME.md rename to plans/completed/PLAN-MCP-FILTER-FRAME.md index ada31c8..d33b75a 100644 --- a/plans/PLAN-MCP-FILTER-FRAME.md +++ b/plans/completed/PLAN-MCP-FILTER-FRAME.md @@ -1,8 +1,10 @@ # Plan: MCP Filter Frame — typed query language migration -Status: **active (planning)**. This plan implements -[`propose/MCP-FILTER-FRAME-PROPOSE.md`](../propose/MCP-FILTER-FRAME-PROPOSE.md) -as a 3-PR sequence. This file is plan-only and does not implement code. +Status: **completed — shipped via PR-FRAME-1 → PR-FRAME-3** (merged 2026-05). +This plan implemented +[`propose/completed/MCP-FILTER-FRAME-PROPOSE.md`](../../propose/completed/MCP-FILTER-FRAME-PROPOSE.md) +as a 3-PR sequence. Per-PR Cursor prompts: +[`CURSOR-PROMPTS-MCP-FILTER-FRAME.md`](CURSOR-PROMPTS-MCP-FILTER-FRAME.md). Depends on: **none** (builds on already-shipped #122 — `extra="forbid"` + per-kind applicability validation). @@ -395,6 +397,6 @@ Landing order: **FRAME-1 → FRAME-2 → FRAME-3**. # Tracking -- `PR-FRAME-1`: _pending_ -- `PR-FRAME-2`: _pending_ -- `PR-FRAME-3`: _pending_ +- `PR-FRAME-1`: merged +- `PR-FRAME-2`: merged +- `PR-FRAME-3`: merged (#133) diff --git a/propose/MCP-FILTER-FRAME-PROPOSE.md b/propose/completed/MCP-FILTER-FRAME-PROPOSE.md similarity index 98% rename from propose/MCP-FILTER-FRAME-PROPOSE.md rename to propose/completed/MCP-FILTER-FRAME-PROPOSE.md index 1b69486..d351eec 100644 --- a/propose/MCP-FILTER-FRAME-PROPOSE.md +++ b/propose/completed/MCP-FILTER-FRAME-PROPOSE.md @@ -1,6 +1,12 @@ # MCP Filter Frame — typed query language with one named carve-out -**Status**: draft +**Status**: **completed — shipped via PR-FRAME-1 → PR-FRAME-3** (merged 2026-05). +Moved to `propose/completed/` once the 3-PR migration landed. The +implementable plan lives at +[`plans/completed/PLAN-MCP-FILTER-FRAME.md`](../../plans/completed/PLAN-MCP-FILTER-FRAME.md); +per-PR Cursor prompts at +[`plans/completed/CURSOR-PROMPTS-MCP-FILTER-FRAME.md`](../../plans/completed/CURSOR-PROMPTS-MCP-FILTER-FRAME.md). + **Author**: Dmitriy Teriaev + Computer **Date**: 2026-05-14 **Issue**: #117 diff --git a/server.py b/server.py index 3855dbd..0b960ff 100644 --- a/server.py +++ b/server.py @@ -329,7 +329,18 @@ async def run_refresh_pipeline(*, quiet: bool = False) -> RefreshIndexOutput: def create_mcp_server() -> FastMCP: mcp = FastMCP("java-codebase-rag", instructions=_INSTRUCTIONS) - @mcp.tool(name="search", description="locate nodes by NL/code text") + @mcp.tool( + name="search", + description=( + "Ranked chunk retrieval: `query` is opaque text (natural language or code fragments); " + "results are score-ranked, not boolean-matched. Optional `filter` uses the same NodeFilter " + "schema as `find` but only **symbol-applicable** fields apply (strict frame). Wildcards " + "(`*`, `?`) in prefix fields are rejected—use ranked `query` text instead. There is **no** " + "structured DSL inside `query`; structured predicates belong in `find`. For " + "identifier-shaped lookups without an exact symbol id/FQN, use `search(query=…)` and " + "`describe` on promising candidates until a dedicated `resolve` tool exists." + ), + ) async def search( query: str = Field(description="Search query"), table: Literal["java", "sql", "yaml", "all"] = Field( @@ -349,9 +360,8 @@ async def search( filter: dict[str, Any] | str | None = Field( default=None, description=( - "Optional NodeFilter (symbol applicability). Unknown keys and populated non-symbol fields return success=false " - "with a teaching message. " - "Prefer a JSON object; a JSON-encoded string is accepted as a fallback." + "Optional NodeFilter post-filter on symbol-oriented hit rows. Unknown keys or populated fields not " + "applicable to symbols return success=false. Prefer a JSON object; a JSON-encoded string is accepted." ), ), ) -> mcp_v2.SearchOutput: @@ -367,7 +377,17 @@ async def search( None, ) - @mcp.tool(name="find", description="locate nodes by structured filter") + @mcp.tool( + name="find", + description=( + "Exact structured listing for one node kind. Per-kind applicable fields: **symbol** — " + "microservice, module, role, exclude_roles, annotation, capability, fqn_prefix, symbol_kind, symbol_kinds; " + "**route** — microservice, module, http_method, path_prefix, framework; **client** — microservice, module, " + "source_layer, client_kind, target_service, target_path_prefix, http_method. " + "Wildcards in prefix fields are rejected. An empty filter (`{}`) or `filter=None` means no predicate (all nodes of " + "that kind; use pagination). Unknown keys or inapplicable populated fields return success=false." + ), + ) async def find( kind: Literal["symbol", "route", "client"] = Field( description=( @@ -378,10 +398,8 @@ async def find( filter: dict[str, Any] | str = Field( ..., description=( - "Required NodeFilter (shared schema, strict extras). Unknown keys and populated fields not applicable to " - "the selected kind return success=false with a teaching message. Symbol filters also support symbol_kind " - "and symbol_kinds. " - "Prefer a JSON object; a JSON-encoded string is accepted as a fallback." + "Required NodeFilter dict (extra keys forbidden). Fields must be applicable to `kind`. " + "Prefer a JSON object; a JSON-encoded string is accepted." ), ), limit: int = Field(default=25, ge=1, le=500, description="Max nodes to return"), @@ -392,12 +410,13 @@ async def find( @mcp.tool( name="describe", description=( - "full record + edge_summary: in/out per stored edge label; " - "type Symbols may add composed keys DECLARES.DECLARES_CLIENT, DECLARES.EXPOSES " - "(describe-time 2-hop member summaries; not valid in neighbors edge_types); " - "method Symbols may add override-axis virtual keys OVERRIDDEN_BY, " - "OVERRIDDEN_BY.DECLARES_CLIENT, OVERRIDDEN_BY.EXPOSES, OVERRIDES (same restriction). " - "Pass id for any node kind, or fqn as an alternative identifier for Symbol nodes only." + "Full node record plus `edge_summary` (in/out counts per stored edge label). Type Symbols may add " + "describe-time composed keys such as DECLARES.DECLARES_CLIENT and DECLARES.EXPOSES; method Symbols may " + "add override-axis virtual keys (OVERRIDDEN_BY, OVERRIDDEN_BY.DECLARES_CLIENT, OVERRIDDEN_BY.EXPOSES, " + "OVERRIDES). Those dot-keys are read-only summaries—not valid `neighbors(edge_types=…)` values. " + "Pass `id` for any kind, or exact `fqn` for Symbol lookup (`id` wins when both are set). " + "For identifier-shaped lookups without an exact id/FQN, use `search(query=…)` then `describe` per candidate " + "until `resolve` ships." ), ) async def describe( @@ -416,7 +435,15 @@ async def describe( ) -> mcp_v2.DescribeOutput: return await asyncio.to_thread(mcp_v2.describe_v2, id, fqn, None) - @mcp.tool(name="neighbors", description="one-hop walk; REQUIRED direction + edge_types") + @mcp.tool( + name="neighbors", + description=( + "One-hop graph walk: **direction** (`in` | `out`) and non-empty **edge_types** are required. " + "Optional `filter` applies to each neighbor endpoint row; populated fields must be applicable to that " + "neighbor's kind—mixed-kind result sets fail on the first inapplicable neighbor (strict frame). " + "Wildcards in prefix fields are rejected. Unknown NodeFilter keys return success=false." + ), + ) async def neighbors( ids: str | list[str] = Field(description="Origin symbol/route/client id, or list for batch"), direction: Literal["in", "out"] = Field( @@ -440,10 +467,8 @@ async def neighbors( filter: dict[str, Any] | str | None = Field( default=None, description=( - "Optional NodeFilter applied to the other endpoint of each edge. Unknown keys and populated fields not " - "applicable to an evaluated neighbor kind return success=false with a teaching message. For mixed " - "neighbor kinds, evaluation fails on the first inapplicable row. " - "Prefer a JSON object; a JSON-encoded string is accepted as a fallback." + "Optional NodeFilter on the neighbor node. Same applicability rules as `find` for that node's kind. " + "Prefer a JSON object; a JSON-encoded string is accepted." ), ), ) -> mcp_v2.NeighborsOutput: diff --git a/tests/test_mcp_v2.py b/tests/test_mcp_v2.py index 9e9bc93..2da454c 100644 --- a/tests/test_mcp_v2.py +++ b/tests/test_mcp_v2.py @@ -10,6 +10,7 @@ NodeFilter, _NODEFILTER_APPLICABLE_FIELDS, describe_v2, + filter_frame_counters, find_v2, neighbors_v2, search_v2, @@ -788,3 +789,33 @@ def test_empty_filter_returns_full_result_set(kuzu_graph) -> None: out = find_v2("client", {}, graph=kuzu_graph) assert out.success is True assert out.results + + +def test_fail_loud_counter_increments_on_applicability_error(kuzu_graph) -> None: + before = filter_frame_counters().get("applicability", 0) + out = find_v2("symbol", {"path_prefix": "/api"}, graph=kuzu_graph) + assert out.success is False + assert filter_frame_counters().get("applicability", 0) == before + 1 + + +def test_fail_loud_counter_increments_on_wildcard_rejection(kuzu_graph) -> None: + before = filter_frame_counters().get("wildcard", 0) + out = find_v2("symbol", {"fqn_prefix": "com.foo.*"}, graph=kuzu_graph) + assert out.success is False + assert filter_frame_counters().get("wildcard", 0) == before + 1 + + +def test_fail_loud_counter_categories_are_distinct(kuzu_graph) -> None: + b_app = filter_frame_counters().get("applicability", 0) + b_wild = filter_frame_counters().get("wildcard", 0) + find_v2("symbol", {"path_prefix": "/x"}, graph=kuzu_graph) + find_v2("symbol", {"fqn_prefix": "com.*"}, graph=kuzu_graph) + assert filter_frame_counters().get("applicability", 0) == b_app + 1 + assert filter_frame_counters().get("wildcard", 0) == b_wild + 1 + + +def test_fail_loud_counter_survives_multiple_calls(kuzu_graph) -> None: + before = filter_frame_counters().get("applicability", 0) + find_v2("symbol", {"http_method": "GET"}, graph=kuzu_graph) + find_v2("symbol", {"http_method": "GET"}, graph=kuzu_graph) + assert filter_frame_counters().get("applicability", 0) >= before + 2