From a0e52e4021503fe86cca0b20657e48eb4071d6f1 Mon Sep 17 00:00:00 2001 From: Dmitry Teryaev Date: Sat, 11 Jul 2026 13:11:22 +0300 Subject: [PATCH] fix(cli): resolve find/search/rendering bugs from CLI QA pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes 13 bugs/gaps found in a manual QA pass of the jrag / java-codebase-rag CLIs against the bank-chat-system fixture, with targeted unit tests. find node-mapping (F10 HIGH / F1 MED): - Enrich NodeRef with the identity fields the envelope contract expects and rewrite _node_ref_from_row so each kind's fqn is its natural identifier (route "METHOD path" / "topic:"; client "member_fqn->target_service"; producer "topic:"), plus name/file. Fixes `find --kind route|client|producer` emitting {"kind":...} / node-N garbage and filter-mode symbols missing name/file. Member-less clients no longer interpolate "None->". search (F6 / F7 / F8): - vector_display_score: bounded linear map so a correctly-ranked top hit never shows score=0.000 (the old 1-d²/2 map clamped to 0 past √2). - Silence LanceDB hybrid autoprojection deprecation warnings via an fd-2 redirect serialized under a module lock (safe under the MCP server's asyncio.to_thread pool); real errors/tracebacks preserved. - declaration_line_number: anchor hits on the type declaration line, not the chunk's package/import line; comment-aware (skip Javadoc/line/block comments). rendering / cli (F3 / F4 / fuzzy): - jrag topics: per-line rendering with composed file (rich list-of-dicts now multi-line); drop the constant "direction" field. - producer headline: "-> (dynamic topic)" for unresolved variable refs. - jrag search --fuzzy: silent no-op (was always-rejected error). analysis / cli (F9 / F11): - analyze-pr: routes_touched as "METHOD path" / "topic:"; blast_radius_by_symbol keyed by FQN (no raw graph ids). - unresolved-calls: drop redundant caller_id (keeps caller_fqn + ucs id). - erase: accept --verbose (parity with other lifecycle commands). docs: document all 31 jrag commands in the CLI playbook; README map --by module. tests: direct unit tests for the new search helpers; find/fuzzy tests updated to the new contracts. Co-Authored-By: Claude --- README.md | 2 +- docs/JAVA-CODEBASE-RAG-CLI.md | 95 +++++++++++- src/java_codebase_rag/analysis/pr_analysis.py | 36 ++++- src/java_codebase_rag/cli.py | 17 ++- src/java_codebase_rag/graph/graph_types.py | 131 +++++++++++++--- src/java_codebase_rag/jrag.py | 70 ++++++--- src/java_codebase_rag/jrag_render.py | 44 +++++- src/java_codebase_rag/mcp/mcp_v2.py | 3 +- .../search/search_lancedb.py | 144 +++++++++++++++++- .../search/search_scoring.py | 77 ++++++++++ tests/mcp/test_mcp_v2.py | 21 ++- tests/package/test_jrag_orientation.py | 23 +-- tests/search/test_search_lancedb.py | 104 ++++++++++++- tests/search/test_search_scoring.py | 85 +++++++++++ 14 files changed, 769 insertions(+), 83 deletions(-) create mode 100644 tests/search/test_search_scoring.py diff --git a/README.md b/README.md index f0f6576..8bbb26c 100644 --- a/README.md +++ b/README.md @@ -98,7 +98,7 @@ Full schemas, `NodeFilter` / `EdgeFilter` semantics, and the hints contract live jrag status # index health (ontology version, freshness, counts) jrag microservices # microservices with resolved type counts jrag map # counts per kind per service/module -jrag map --module # group by module instead +jrag map --by module # group by module instead (--module filters) jrag conventions # dominant roles + framework tallies jrag overview chat-core # bundle for a microservice jrag overview /chat/assign # route flow (inbound callers + outbound CALLS) diff --git a/docs/JAVA-CODEBASE-RAG-CLI.md b/docs/JAVA-CODEBASE-RAG-CLI.md index 20b66e7..7d93ad7 100644 --- a/docs/JAVA-CODEBASE-RAG-CLI.md +++ b/docs/JAVA-CODEBASE-RAG-CLI.md @@ -355,9 +355,100 @@ Prefer **`java-codebase-rag reprocess --graph-only`** when you only need Ladybug - [CODEBASE_REQUIREMENTS.md](./CODEBASE_REQUIREMENTS.md) — repo layout, brownfield, when to rebuild. - [MANUAL-VERIFICATION-CHECKLIST.md](./MANUAL-VERIFICATION-CHECKLIST.md) — phased checks that mix CLI + MCP. -## `jrag` command — search CLI +## `jrag` command — agent CLI -The **`jrag`** command provides semantic search over the LanceDB index. This is the CLI surface for search (see MCP `search` tool in AGENT-GUIDE.md for the programmatic interface). +`jrag` is the **agent-facing** CLI — a separate console script alongside `java-codebase-rag`. It exposes **one command per engineering intent** over the same LanceDB vectors + LadybugDB graph as the MCP surface, and takes human-readable identifiers (FQN / simple name / route path / topic) — never raw node IDs. Every `` command resolves the identifier as its first step; on `many` candidates it returns them and stops, on `none` it returns `not_found` (auto-pick is forbidden). + +Output defaults to compact text; `--format json` emits the shared envelope verbatim. Shared flags apply to most commands: `--service`, `--module`, `--limit`, `--format`, `--detail {brief,normal,full}`, `--index-dir`. `` commands also take resolve hints (`--kind`, `--role`, `--fqn-contains`). Run `jrag --help` for the per-command synopsis; for the full envelope contract and flag table, see [`jrag` — agent CLI](../README.md#jrag--agent-cli) in the README. + +A missing or stale index produces an actionable `status: error` envelope (exit **2**) rather than a traceback. + +### Command reference + +Commands are grouped by engineering intent. Every `` argument is a human-readable identifier (FQN, `Symbol#method(args)`, route path, or topic), resolved before any graph walk. + +#### Orientation + +Index health and coarse-grained maps — no identifier needed. + +```bash +jrag status # index freshness, ontology version, counts +jrag microservices # microservices with resolved type counts +jrag map # symbol counts per kind, grouped by microservice +jrag map --by module # ...grouped by module instead +jrag conventions # dominant roles + framework tallies +jrag overview chat-core # bundle for a microservice +jrag overview /chat/assign # route flow (inbound callers + outbound CALLS) +jrag overview banking.chat # topic: producers + consumers +jrag overview chat-core --as microservice # override auto-detection +``` + +`overview` dispatches on its subject: a `/`-prefixed string is a route (same as `flow`), a known microservice name yields its routes + clients + producers, otherwise it is treated as a topic. `--as {microservice,route,topic}` overrides the auto-detection. + +#### Locate + +Find a specific node by name, or inspect one in full. `` commands resolve first. + +```bash +jrag find ChatService # exact name / FQN lookup (symbols only) +jrag find --role CONTROLLER # filter mode (structured NodeFilter flags) +jrag find --framework spring_mvc --capability HTTP_CLIENT +jrag inspect ChatService # full node record + edge summary +jrag outline src/main/.../Foo.java # symbols declared in a file +jrag imports src/main/.../Foo.java # imports resolved to graph nodes +``` + +`find` has two modes: a positional `` for exact name/FQN lookup (symbols only), or **filter mode** (no positional) using structured flags (`--role`, `--java-kind`, `--annotation`, `--capability`, `--framework`, `--http-method`, `--client-kind`, `--producer-kind`, `--topic-contains`, …). Domain flags imply `--kind` when omitted; `--offset` paginates in filter mode only. + +#### Listings + +Surface every node of a given Java role. No ``; pair with `--service` / `--module` to scope. + +```bash +jrag http-routes # HTTP routes +jrag http-clients # HTTP clients (Feign / RestTemplate / WebClient) +jrag producers # async message producers (Kafka / StreamBridge) +jrag topics # message topics, grouped by producer +jrag jobs # scheduled tasks (@Scheduled) +jrag listeners # message listeners (@KafkaListener etc.) +jrag entities # JPA entities +``` + +#### Traversals + +One-hop and multi-hop walks. **All resolve-first** — pass a human-readable identifier. + +```bash +jrag callers ChatService#assign(Request) # who calls me? +jrag callers ChatIngressController # a controller also lists its EXPOSES routes +jrag callees ChatService#assign(Request) # what do I call? +jrag dependencies ChatService # types this Symbol injects (INJECTS out) +jrag dependents PaymentGateway # who injects this type? +jrag hierarchy AbstractBase # type tree (parents + children) +jrag implementations PaymentProcessor # classes implementing an interface +jrag subclasses AbstractRepository # classes extending a type +jrag overrides Impl#run() # methods this overrides (dispatch UP) +jrag overridden-by Iface#run() # methods overriding this (dispatch DOWN) +jrag impact PaymentGateway # fleet-wide blast radius (reverse closure) +jrag decompose ChatIngressController#assign # role-waterfall flow from an entrypoint +jrag flow /chat/assign # request flow through a route +jrag connection chat-core # cross-service connections (inbound/outbound) +``` + +#### Semantic search + +```bash +jrag search "assign a chat agent" # semantic over Lance (java table) +jrag search "kafka" --table all # java + sql + yaml tables +``` + +See [`jrag search`](#jrag-search) below for the full flag reference (hybrid, explain, dedup, pagination, role/framework filters, generated-source filtering). + +#### Maintenance + +```bash +jrag vocab-index # rebuild the vocabulary sidecar (did-you-mean / absence diagnosis) +``` ### `jrag search` diff --git a/src/java_codebase_rag/analysis/pr_analysis.py b/src/java_codebase_rag/analysis/pr_analysis.py index 34ff778..71ec8ec 100644 --- a/src/java_codebase_rag/analysis/pr_analysis.py +++ b/src/java_codebase_rag/analysis/pr_analysis.py @@ -377,6 +377,31 @@ def _route_ids_for_symbol(graph: Any, symbol_id: str) -> list[str]: return out +def _route_natural_id(graph: Any, rid: str) -> str: + """Map a raw Route node id to its agent-facing natural identifier. + + Mirrors the envelope contract (``METHOD path`` for HTTP endpoints, + ``topic:`` for kafka topics surfaced as :Route) so ``routes_touched`` + in the PR risk report is readable instead of leaking raw graph ids like + ``r:970ffaa960a4f65d``. Falls back to ``rid`` only if the route vanished. + """ + rows = graph._rows( + "MATCH (r:Route {id: $rid}) " + "RETURN r.method AS method, r.path_template AS path_template, " + "r.path AS path, r.topic AS topic LIMIT 1", + {"rid": rid}, + ) + if not rows: + return rid + r = rows[0] + method = str(r.get("method") or "") + path = str(r.get("path_template") or r.get("path") or "") + if method or path: + return f"{method} {path}".strip() + topic = str(r.get("topic") or "") + return f"topic:{topic}" if topic else rid + + def compute_risk(graph: Any, changed: list[ChangedSymbol]) -> PrRiskReport: """Aggregate blast radius, routes, cross-service callers, and v1 risk score. @@ -415,7 +440,9 @@ def compute_risk(graph: Any, changed: list[ChangedSymbol]) -> PrRiskReport: needle = _impact_needle_for_changed(graph, fqn, cs.kind) ia = graph.impact_analysis(needle, depth=2, limit=400) n = len(ia) - blast_by[cs.symbol_id] = n + # Key blast radius by the symbol's FQN (agent-facing identifier), not + # its raw graph id — the report is operator-facing JSON. + blast_by[fqn or cs.symbol_id] = n blast_total += n for e in graph.find_callers(cs.fqn, depth=2, limit=400): @@ -429,8 +456,11 @@ def compute_risk(graph: Any, changed: list[ChangedSymbol]) -> PrRiskReport: cs_cross_service = 0 route_ids = _route_ids_for_symbol(graph, cs.symbol_id) for rid in route_ids: - if rid not in routes: - routes.append(rid) + # Record the route by its natural identifier (METHOD path / + # topic:name), not the raw graph id. + label = _route_natural_id(graph, rid) + if label and label not in routes: + routes.append(label) callers = graph._rows( "MATCH (s:Symbol)-[:DECLARES_CLIENT]->(c:Client)-[e:HTTP_CALLS]->(r:Route {id: $rid}) " "WHERE e.match = 'cross_service' " diff --git a/src/java_codebase_rag/cli.py b/src/java_codebase_rag/cli.py index 54a4023..39cdfbf 100644 --- a/src/java_codebase_rag/cli.py +++ b/src/java_codebase_rag/cli.py @@ -846,7 +846,15 @@ def _cmd_unresolved_calls_list(args: argparse.Namespace) -> int: callee_simple=args.callee_simple, limit=int(args.limit), ) - _emit({"success": True, "count": len(rows), "sites": rows}) + # Drop the raw caller symbol id: the row already carries the agent-facing + # ``caller_fqn``, so ``caller_id`` is redundant noise in an operator-facing + # report. The call-site ``id`` (``ucs:``) is kept — it's each site's + # primary key, not a caller reference. + sites = [ + {k: v for k, v in row.items() if k != "caller_id"} + for row in rows + ] + _emit({"success": True, "count": len(sites), "sites": sites}) return 0 @@ -1060,12 +1068,7 @@ def build_parser() -> argparse.ArgumentParser: ) _add_index_embedding_flags(erase) erase.add_argument("--yes", action="store_true", help="Confirm destructive deletion (required in CI)") - erase.add_argument( - "--quiet", "-q", - action="store_true", - dest="quiet", - help="Suppress stderr progress relay; stdout payload unchanged.", - ) + _add_verbosity_flags(erase) erase.set_defaults(handler=_cmd_erase) meta = subparsers.add_parser("meta", help="Print graph meta and embedding resolution.") diff --git a/src/java_codebase_rag/graph/graph_types.py b/src/java_codebase_rag/graph/graph_types.py index d067f12..dba7225 100644 --- a/src/java_codebase_rag/graph/graph_types.py +++ b/src/java_codebase_rag/graph/graph_types.py @@ -36,6 +36,24 @@ class NodeRef(BaseModel): role: str | None = None generated: bool | None = None generated_by: str | None = None + # Identity-adjacent fields consumed by ``jrag_envelope.node_key`` (keying) + # and ``project_node`` allow-lists (rendering) so find filter-mode nodes + # carry the same rich shape as the dedicated listings (http-routes / + # http-clients / producers). All optional with default None: existing + # NodeRef constructors are unaffected and ``_drop_empty`` strips unset + # fields at the JSON boundary. + method: str | None = None + path: str | None = None + framework: str | None = None + member_fqn: str | None = None + target_service: str | None = None + client_kind: str | None = None + topic: str | None = None + broker: str | None = None + producer_kind: str | None = None + filename: str | None = None + start_line: int | None = None + resolved: bool | None = None class StructuredHint(BaseModel): @@ -98,38 +116,107 @@ def _resolve_node_kind( def _node_ref_from_row(kind: Literal["symbol", "route", "client", "producer"], row: dict[str, Any]) -> NodeRef: - symbol_kind: str | None = None + """Map a graph store row to a :class:`NodeRef`. + + ``fqn`` is set to each kind's documented natural identifier (README + §"jrag — agent CLI") so ``jrag_envelope.node_key`` (which checks ``fqn`` + first) keys nodes correctly and no raw graph id leaks: + + * symbol -> symbol fqn + * route -> ``"METHOD path"`` (HTTP endpoint); ``"topic:"`` when a + kafka topic surfaces as :Route; ``""`` for a phantom route + (node_key then falls through to the composed ``file``) + * client -> ``"member_fqn->target_service"`` + * producer-> ``"topic:"`` + + Rich detail (member_fqn / target_service / method / path / topic / + framework / filename / start_line / resolved) is populated alongside so + find filter-mode nodes render with the same shape as the dedicated + listings (``http-routes`` / ``http-clients`` / ``producers``) instead of + collapsing to ``{"kind": ...}`` after the envelope's projection+drop-empty. + """ + microservice = str(row.get("microservice") or "") or None + module = str(row.get("module") or "") or None + filename = str(row.get("filename") or "") or None + start_line = row.get("start_line") + try: + start_line = int(start_line) if start_line not in (None, "") else None + except (TypeError, ValueError): + start_line = None + resolved_raw = row.get("resolved") + resolved = bool(resolved_raw) if resolved_raw is not None else None + nid = str(row.get("id") or "") + if kind == "symbol": fqn = str(row.get("fqn") or "") role = str(row.get("role") or "") or None symbol_kind_val = str(row.get("symbol_kind") or row.get("kind") or "").strip() symbol_kind = symbol_kind_val or None - elif kind == "route": + return NodeRef( + id=nid, kind="symbol", fqn=fqn, + name=str(row.get("name") or "") or None, + symbol_kind=symbol_kind, + microservice=microservice, module=module, role=role, + filename=filename, start_line=start_line, + generated=bool(row.get("generated")) if row.get("generated") is not None else None, + generated_by=str(row.get("generated_by")) if row.get("generated_by") else None, + ) + + if kind == "route": method = str(row.get("method") or "") path = str(row.get("path_template") or row.get("path") or "") - fqn = f"{method} {path}".strip() - role = None - elif kind == "client": + topic = str(row.get("topic") or "") or None + if method or path: + fqn = f"{method} {path}".strip() + elif topic: + fqn = f"topic:{topic}" + else: + fqn = "" + return NodeRef( + id=nid, kind="route", fqn=fqn, name=(path or topic), + method=method or None, path=path or None, topic=topic, + framework=str(row.get("framework") or "") or None, + microservice=microservice, module=module, + filename=filename, start_line=start_line, resolved=resolved, + ) + + if kind == "client": + mfqn = str(row.get("member_fqn") or "") + tgt = str(row.get("target_service") or "") method = str(row.get("method") or "") - target = str(row.get("target_service") or "") path = str(row.get("path_template") or row.get("path") or "") - fqn = f"{target} {method} {path}".strip() - role = None - else: - topic = str(row.get("topic") or "") - broker = str(row.get("broker") or "") - fqn = f"{topic} {broker}".strip() - role = None + member_fqn = mfqn or None + target_service = tgt or None + member_simple = (mfqn.rsplit(".", 1)[-1] if mfqn else "") or None + # Build the contract id ``member_fqn->target_service`` from the RAW + # strings: ``member_fqn`` was normalized to None above, so interpolating + # it would emit ``"None->"`` for member-less (brownfield/meta) + # clients. Fall back to whichever half is present; never a dangling arrow. + fqn = f"{mfqn}->{tgt}" if (mfqn and tgt) else (mfqn or tgt) + return NodeRef( + id=nid, kind="client", + fqn=fqn, + name=member_simple, + member_fqn=member_fqn, target_service=target_service, + method=method or None, path=path or None, + client_kind=str(row.get("client_kind") or "") or None, + microservice=microservice, module=module, + filename=filename, start_line=start_line, resolved=resolved, + ) + + # producer + topic = str(row.get("topic") or "") or None + member_fqn = str(row.get("member_fqn") or "") or None + member_simple = (member_fqn.rsplit(".", 1)[-1] if member_fqn else "") or None return NodeRef( - id=str(row.get("id") or ""), - kind=kind, - fqn=fqn, - symbol_kind=symbol_kind, - microservice=str(row.get("microservice") or "") or None, - module=str(row.get("module") or "") or None, - role=role, - generated=bool(row.get("generated")) if row.get("generated") is not None else None, - generated_by=str(row.get("generated_by")) if row.get("generated_by") else None, + id=nid, kind="producer", + fqn=(f"topic:{topic}" if topic else ""), + name=(topic or member_simple), + topic=topic, broker=str(row.get("broker") or "") or None, + producer_kind=str(row.get("producer_kind") or "") or None, + member_fqn=member_fqn, + microservice=microservice, module=module, + filename=filename, start_line=start_line, resolved=resolved, ) diff --git a/src/java_codebase_rag/jrag.py b/src/java_codebase_rag/jrag.py index bbb43ff..0383607 100644 --- a/src/java_codebase_rag/jrag.py +++ b/src/java_codebase_rag/jrag.py @@ -1108,9 +1108,9 @@ def _core_parser() -> argparse.ArgumentParser: "--table all searches all three. --hybrid enables vector+keyword hybrid. " "--offset paginates. --path-contains narrows by file path substring. " "Filters (NodeFilter flags) narrow results.\n\n" - "--fuzzy is accepted but rejected IN-HANDLER with status: error (search is " - "inherently semantic; --fuzzy is a no-op synonym). Registering the flag " - "prevents argparse from exiting 2 before the handler can produce the envelope." + "--fuzzy is accepted as a no-op (search is inherently semantic; " + "--fuzzy is implicit). It is kept registered so callers that pass " + "it don't hit an argparse error, and is silently ignored." ), ) search.add_argument("query", help="Natural-language search query.") @@ -1132,7 +1132,7 @@ def _core_parser() -> argparse.ArgumentParser: ) search.add_argument( "--fuzzy", action="store_true", - help="Accepted but rejected in-handler (search is semantic; --fuzzy is implicit).", + help="Accepted as a no-op (search is always semantic; --fuzzy is implicit).", ) search.add_argument( "--min-score", type=float, default=0.0, dest="min_score", @@ -1874,6 +1874,43 @@ def _cmd_producers(args: argparse.Namespace) -> int: return _render_listing(rows, limit=limit, args=args, noun="producer") +def _producer_summary(producer: dict) -> dict: + """Display-oriented producer dict for the ``topics`` grouping. + + The raw ``list_producers`` row carries 11 fields (incl. empty ``broker``, + raw ``filename``/``start_line``/``end_line``, ``direction``, ``source_layer``) + which the text renderer used to collapse into one unreadable comma-line. + This folds location into a single ``file`` and keeps only the fields an + operator needs to identify the producer under a topic header (the topic + itself is the group header, so it's dropped here). Empty values omitted. + """ + member_fqn = str(producer.get("member_fqn") or "") + member_simple = member_fqn.rsplit(".", 1)[-1] if member_fqn else "" + filename = str(producer.get("filename") or "") + start_line = producer.get("start_line") + try: + sl = int(start_line) if start_line not in (None, "") else None + except (TypeError, ValueError): + sl = None + file_loc = f"{filename}:{sl}" if filename and sl else filename + out: dict = {} + if member_simple: + out["member"] = member_simple + if file_loc: + out["file"] = file_loc + # ``direction`` is intentionally omitted: every Producer is built with + # direction="producer" (build_ast_graph), so it's a constant that only + # inflates each block with zero information. + for k in ("microservice", "module", "producer_kind", "broker"): + v = producer.get(k) + if v not in (None, "", []): + out[k] = v + resolved = producer.get("resolved") + if resolved is not None: + out["resolved"] = bool(resolved) + return out + + def _cmd_topics(args: argparse.Namespace) -> int: from java_codebase_rag.jrag_envelope import Envelope, mark_truncated, next_actions_hook from java_codebase_rag.jrag_render import render @@ -1908,7 +1945,7 @@ def _cmd_topics(args: argparse.Namespace) -> int: "producers": [], "broker": producer.get("broker") or "", } - topics_dict[topic]["producers"].append(producer) + topics_dict[topic]["producers"].append(_producer_summary(producer)) warnings: list[str] = [] if no_topic_count: @@ -3743,8 +3780,8 @@ def _cmd_imports(args: argparse.Namespace) -> int: # (kv-block + nested dict sections) so the agent sees compact structured data. # # Search dispatches to search_v2 (mcp_v2.search_v2) after building a NodeFilter -# from flags. --fuzzy is registered on the parser but rejected IN-HANDLER with -# status: error (not argparse exit) so the envelope carries the message. +# from flags. --fuzzy is registered on the parser and accepted as a silent +# no-op (search is inherently semantic; --fuzzy is implicit). # ============================================================================ @@ -4247,24 +4284,19 @@ def _cmd_search(args: argparse.Namespace) -> int: """search — semantic search via search_v2 over Lance tables. Builds a NodeFilter from flags, calls search_v2 with limit+1 for +1-fetch - truncation, and renders. --fuzzy is rejected IN-HANDLER (not argparse-exit) - so the error carries the canonical envelope shape. + truncation, and renders. --fuzzy is accepted as a silent no-op (search is + always semantic; --fuzzy is implicit). """ from java_codebase_rag.mcp import mcp_v2 from java_codebase_rag.jrag_envelope import Envelope, mark_truncated, next_actions_hook, normalize_enum from java_codebase_rag.jrag_render import render - # --fuzzy: registered on the parser (so argparse doesn't exit 2), but rejected - # IN-HANDLER with status: error (search is inherently semantic; --fuzzy is - # a no-op synonym, not a real mode toggle). - if getattr(args, "fuzzy", False): - env = Envelope( - status="error", - message="search is semantic; --fuzzy is implicit", - ) - print(render(env, fmt=args.format, detail=args.detail)) - return 2 + # --fuzzy: accepted as a silent no-op. Search is inherently semantic + # (vector + lexical), so --fuzzy is implicit; the flag is kept registered + # so callers/agents that pass it don't hit an argparse or envelope error, + # and is simply ignored here. + _ = getattr(args, "fuzzy", False) cfg, graph, rc = _load_graph_or_error(args) if rc: diff --git a/src/java_codebase_rag/jrag_render.py b/src/java_codebase_rag/jrag_render.py index 5c4ab0a..0dafdbd 100644 --- a/src/java_codebase_rag/jrag_render.py +++ b/src/java_codebase_rag/jrag_render.py @@ -125,6 +125,23 @@ def _next_action_lines(envelope: Envelope) -> list[str]: return [f"next: {hint}" for hint in envelope.agent_next_actions[:2]] +def _is_dynamic_topic_ref(topic: str) -> bool: + """True when a producer ``topic`` string is a bare Java identifier — a + variable or method-call name the indexer captured because it could not + resolve the destination at index time — rather than a real topic name. + + Real topics are dotted (``banking.chat.audit``) or CONSTANT references + (``ChatTopics.ESCALATION``, ``OPERATOR_NOTIFICATIONS``); both contain a + ``.`` or ``_``. A single lowercase/identifier token (``topic``, + ``distributionTopic``) is a runtime reference. Used by :func:`display_name` + only when the producer is also ``resolved=False`` — a resolved bare token is + treated as a (rare) real single-word topic. + """ + if "." in topic or "_" in topic: + return False + return bool(topic) and topic[0].islower() and topic.isalnum() + + def display_name(node: dict[str, Any]) -> str: """Best short label for a node across all kinds (symbol + route/client/producer). @@ -175,6 +192,15 @@ def display_name(node: dict[str, Any]) -> str: base = member_fqn.rsplit(".", 1)[-1] topic = str(node.get("topic") or "").strip() if topic: + # An unresolved topic that is a bare Java identifier is a runtime + # reference the indexer could not resolve (e.g. the variable + # ``topic``, or ``getKafka().getDistributionTopic()`` reduced to + # ``distributionTopic``). Printing it verbatim would show + # ``→ topic`` and mislead the agent into treating the variable name + # as the Kafka destination; surface it as dynamic instead. Real + # topic names (dotted / CONSTANT) stay verbatim. + if node.get("resolved") is False and _is_dynamic_topic_ref(topic): + return f"{base} → (dynamic topic)" return f"{base} → {topic}" target = str(node.get("target_service") or "").strip() if target: @@ -619,8 +645,24 @@ def _render_inspect_block(node: dict[str, Any], indent: int) -> list[str]: out.extend(_render_inspect_block(val, indent + 1)) elif _is_dict_list(val): out.append(f"{pad}{key}:") + item_pad = " " * (indent + 1) + # Rich items (>2 keys: e.g. ``topics`` producers) render one kv per + # line for readability — a wall of comma-joined keys is unreadable + # and hides the composed ``file`` location. Short sample items + # (``route_sample`` / ``client_sample``) stay on a single line. + rich = any(len(it) > 2 for it in val) for item in val: - out.append(f"{pad} - {_inspect_inline(item)}") + if rich: + # Render at indent+2 so continuation kvs align under the + # first kv (which sits after the `- ` marker at indent+1). + item_lines = _render_inspect_block(item, indent + 2) + if not item_lines: + out.append(f"{item_pad}- {{}}") + else: + out.append(f"{item_pad}- {item_lines[0].lstrip()}") + out.extend(item_lines[1:]) + else: + out.append(f"{item_pad}- {_inspect_inline(item)}") else: out.append(f"{pad}{key}: {_inspect_inline(val)}") return out diff --git a/src/java_codebase_rag/mcp/mcp_v2.py b/src/java_codebase_rag/mcp/mcp_v2.py index f2e1544..7310054 100644 --- a/src/java_codebase_rag/mcp/mcp_v2.py +++ b/src/java_codebase_rag/mcp/mcp_v2.py @@ -1136,7 +1136,8 @@ def find_v2( ) params["lim"] = fetch_cap rows = g._rows( # noqa: SLF001 - f"MATCH (s:Symbol) {where} RETURN s.id AS id, s.fqn AS fqn, s.microservice AS microservice, " + f"MATCH (s:Symbol) {where} RETURN s.id AS id, s.fqn AS fqn, s.name AS name, " + "s.filename AS filename, s.start_line AS start_line, s.microservice AS microservice, " "s.module AS module, s.role AS role, s.kind AS symbol_kind, s.generated AS generated, s.generated_by AS generated_by ORDER BY s.fqn LIMIT $lim", params, ) diff --git a/src/java_codebase_rag/search/search_lancedb.py b/src/java_codebase_rag/search/search_lancedb.py index 9f74cc7..a4958b6 100644 --- a/src/java_codebase_rag/search/search_lancedb.py +++ b/src/java_codebase_rag/search/search_lancedb.py @@ -7,8 +7,11 @@ import json import os import sys +import tempfile import threading +import warnings from collections.abc import Callable +from contextlib import contextmanager from pathlib import Path import lancedb @@ -44,8 +47,10 @@ _role_weight, _split_identifier, _symbol_bonus, + declaration_line_number, explain_score_components, l2_distance_to_score, + vector_display_score, ) TABLES: dict[str, str] = { @@ -296,6 +301,117 @@ def _combine_predicates(parts: list[str | None]) -> str | None: return " AND ".join(f"({p})" for p in clean) +# LanceDB (0.30.x) emits two Rust `tracing` WARN lines per hybrid query to stderr +# — "specified output columns but did not include `_score`/`_distance` ... Call +# `disable_scoring_autoprojection`". They are noise on the agent's stderr, not +# Python warnings (so `warnings.filterwarnings` can't catch them), and the fluent +# query builder exposes no `disable_scoring_autoprojection()` (the lower-level +# `to_lance().scanner(...)` path needs `pylance`, which isn't installed on the +# PEP 508 graph-only profile). We match them by stable substring so anything that +# is a REAL error still reaches stderr. +_LANCE_AUTOPROJ_MARKERS: tuple[str, ...] = ( + "disable_scoring_autoprojection", + "did not include `_distance`", + "did not include `_score`", +) + +# The fd-2 redirect below mutates the PROCESS-GLOBAL fd 2 (and +# ``warnings.catch_warnings`` mutates global warning state). The MCP server +# dispatches every tool call through ``asyncio.to_thread`` on a thread pool +# (server.py), so two concurrent hybrid/auto-hybrid searches would race on the +# dup2 bookkeeping — corrupting the saved fd and crashing the whole server with +# ``Bad file descriptor``. Serialize the redirect so only one thread mutates fd +# 2 / warning state at a time. Concurrent hybrid queries therefore serialize +# their ``to_list()`` (correctness over throughput); a Rust-tracing-level +# suppression would remove the fd hijack entirely (follow-up). +_LANCE_WARN_REDIRECT_LOCK = threading.Lock() + + +def _is_autoproj_noise(line: str) -> bool: + """True for a LanceDB autoprojection-deprecation line (to drop). + + Preserves genuine errors/tracebacks even if they happen to reference the API + name — only the bare deprecation log lines (no Error/Traceback/Exception) are + treated as noise. + """ + if not any(marker in line for marker in _LANCE_AUTOPROJ_MARKERS): + return False + return not any(seg in line for seg in ("Traceback", "Error:", "error:", "Exception")) + + +@contextmanager +def _silence_lance_autoproj_warnings(): + """Swallow LanceDB's `_score`/`_distance` autoprojection deprecation warnings. + + Redirects fd 2 to a temp buffer for the duration of the wrapped call, drops + only the autoprojection deprecation lines, and re-emits everything else to + the real stderr so genuine errors stay visible. No-op if the caller opted + back in via ``JAVA_CODEBASE_RAG_KEEP_LANCE_WARNINGS`` (debugging). + + Thread-safety: the redirect is serialized under ``_LANCE_WARN_REDIRECT_LOCK`` + because it mutates process-global fd 2 and warning state — the MCP server + runs tool calls concurrently on a thread pool. + """ + if os.environ.get("JAVA_CODEBASE_RAG_KEEP_LANCE_WARNINGS"): + yield + return + # Also catch the (unlikely) Python-warning form defensively. + with _LANCE_WARN_REDIRECT_LOCK, warnings.catch_warnings(): + warnings.filterwarnings( + "ignore", + message=r".*(disable_scoring_autoprojection|did not include `(_distance|_score)`).*", + ) + with tempfile.TemporaryFile(mode="w+", encoding="utf-8", errors="replace") as captured: + saved = os.dup(2) + try: + os.dup2(captured.fileno(), 2) + yield + finally: + # Restore fd 2 FIRST so the re-emit below reaches real stderr. + os.dup2(saved, 2) + os.close(saved) + captured.seek(0) + kept = "".join(line for line in captured if not _is_autoproj_noise(line)) + if kept: + sys.stderr.write(kept) + sys.stderr.flush() + + +def _simple_type_name(fqn: str | None) -> str | None: + """``com.foo.Bar`` -> ``Bar``; None/empty -> None.""" + if not fqn: + return None + return str(fqn).rsplit(".", 1)[-1] or None + + +def _refine_java_start_lines(rows: list[dict]) -> None: + """Point each java row's ``start.line`` at the type declaration, not the chunk anchor. + + LanceDB chunks are anchored at the chunk's first source line — for a + file-spanning chunk that's the package/import line (``start.line`` = 1) + while the ``class``/``interface`` declaration sits several lines down. The + chunk anchor is a poor display line for a symbol hit (renders as + ``File.java:1``); derive the real declaration line from the chunk text + (pinned to the primary type) so a hit shows ``File.java:`` instead + (F8). Method-only chunks whose range doesn't include a type declaration + keep their chunk anchor unchanged. + """ + for r in rows: + if str(r.get("_kind", "")) != "java": + continue + start = r.get("start") + if not isinstance(start, dict): + continue + anchor = start.get("line") + if anchor is None: + continue + hints = r.get("_hints") or {} + type_name = hints.get("primary_type_hint") or _simple_type_name(r.get("primary_type_fqn")) + decl = declaration_line_number(r.get("text"), int(anchor), type_name) + if decl is not None: + start["line"] = decl + + def _search_one_table( table_name: str, *, @@ -342,7 +458,11 @@ def _search_one_table( ) if combined_pred: q = q.where(combined_pred, prefilter=True) - rows = q.to_list() + # Hybrid selects explicit output columns without `_score`/`_distance`, so + # LanceDB (0.30.x) emits two Rust autoprojection deprecation WARNs to + # stderr per query. Silence just those lines; real errors still surface. + with _silence_lance_autoproj_warnings(): + rows = q.to_list() for r in rows: r["_kind"] = kind rs = r.pop("_relevance_score", None) @@ -375,7 +495,11 @@ def _search_one_table( # exposed score was always 0.0, making results look unranked. d = r.get("_distance") if d is not None: - r["_score"] = l2_distance_to_score(float(d)) + # Use the same non-clamping map as the display sites so graph-expand + # rows (which run_search does NOT overwrite) never carry the old + # 1-d²/2 value that collapses to 0 past √2. (The main single/multi + # paths overwrite this with the bonus-adjusted effective distance.) + r["_score"] = vector_display_score(float(d)) r["start"] = coerce_position_field(r.get("start")) r["end"] = coerce_position_field(r.get("end")) return rows @@ -575,6 +699,7 @@ def _graph_expand_merge( except Exception: return vector_rows _apply_chunk_hints(graph_rows) + _refine_java_start_lines(graph_rows) graph_rows.sort(key=_vector_sort_key) for r in graph_rows: r["_graph_expanded"] = True @@ -737,6 +862,9 @@ def run_search( extra_predicates=preds, ) _apply_chunk_hints(rows) + # Anchor each java row's start.line on the type declaration instead of + # the chunk's first source line (often the package/import line = 1). + _refine_java_start_lines(rows) if skip_role_weight: for r in rows: r["_skip_role_weight"] = True @@ -747,11 +875,14 @@ def run_search( _hybrid_post_sort_normalization(rows) else: rows.sort(key=_vector_sort_key) - # Vector: set honest displayed score from adjusted distance, clamped to [0,1] + # Vector: displayed score from the effective (bonus-adjusted) distance, + # normalized over the unit-embedding range so a correctly-ranked top + # hit never collapses to 0.000 (the cosine map 1 - d²/2 clamps to 0 + # past √2; weak-but-best matches commonly sit at d ≈ 1.5). for r in rows: comps = r.setdefault("_score_components", {}) effective_dist = _effective_distance(comps) - r["_score"] = _clamp01(l2_distance_to_score(effective_dist)) + r["_score"] = vector_display_score(effective_dist) if graph_expand and key == "java" and expand_depth > 0: rows = _graph_expand_merge( @@ -792,16 +923,17 @@ def run_search( ) ) _apply_chunk_hints(merged) + _refine_java_start_lines(merged) if skip_role_weight: for r in merged: r["_skip_role_weight"] = True _apply_symbol_bonus(merged, query_toks) merged.sort(key=_vector_sort_key) - # Vector: set honest displayed score from adjusted distance, clamped to [0,1] + # Vector: displayed score from the effective (bonus-adjusted) distance. for r in merged: comps = r.setdefault("_score_components", {}) effective_dist = _effective_distance(comps) - r["_score"] = _clamp01(l2_distance_to_score(effective_dist)) + r["_score"] = vector_display_score(effective_dist) # Dedup by primary_type_fqn after all sorting/merging, before windowing merged = _dedup_by_fqn(merged, dedup_by_fqn=dedup_by_fqn) diff --git a/src/java_codebase_rag/search/search_scoring.py b/src/java_codebase_rag/search/search_scoring.py index 6a48a99..3a30546 100644 --- a/src/java_codebase_rag/search/search_scoring.py +++ b/src/java_codebase_rag/search/search_scoring.py @@ -12,6 +12,7 @@ from __future__ import annotations import json +import re # Over-fetch multiplier for dedup: fetch 4x to absorb per-FQN chunk multiplicity # so that after collapsing by primary_type_fqn, a page stays full and the +1 @@ -210,6 +211,29 @@ def l2_distance_to_score(distance: float) -> float: return 1.0 - distance * distance / 2.0 +# Display-score denominator for the vector backend. Unit-normalized embeddings +# have L2 distance in [0, 2]; the cosine map ``l2_distance_to_score`` (1 - d²/2) +# goes NEGATIVE past √2 ≈ 1.414 and clamps to 0. Weak-but-best semantic matches +# (e.g. a lone keyword like "controller") commonly sit at d ≈ 1.5, so EVERY hit +# clamps to score=0.000 even though the ranking is correct. ``vector_display_score`` +# instead normalizes the effective (bonus-adjusted) distance over the full +# unit-embedding range, so a top-ranked hit stays visibly non-zero. Role/symbol +# bonuses reduce the effective distance and so raise the displayed score, +# keeping it rank-monotonic with the distance-based sort key. +_VECTOR_DISTANCE_REF = 2.0 + + +def vector_display_score(effective_distance: float) -> float: + """Displayed vector score in [0, 1] from the effective (bonus-adjusted) distance. + + Bounded linear normalization over the unit-embedding L2 range [0, 2]: lower + distance → higher score. Unlike ``l2_distance_to_score`` (which goes + negative past √2 and clamps a correctly-ranked top hit to 0.000), this keeps + a top result visibly non-zero while staying rank-monotonic with the sort key. + """ + return _clamp01(1.0 - effective_distance / _VECTOR_DISTANCE_REF) + + def _effective_distance(comps: dict[str, float]) -> float: """Compute the adjusted distance used for sorting. @@ -231,6 +255,59 @@ def _clamp01(x: float) -> float: return x +# Matches a Java top-level type declaration and captures its simple name. Mirrors +# the heuristic in ``ast.chunk_heuristics._JAVA_TYPE`` but is duplicated here so +# this module stays dependency-free (importable on graph-only Intel installs). +_JAVA_TYPE_DECL_RE = re.compile( + r"\b(?:public\s+|private\s+|protected\s+|sealed\s+|non-sealed\s+|final\s+|" + r"abstract\s+|static\s+)*" + r"(?:class|interface|enum|record)\s+([A-Za-z_][A-Za-z0-9_]*)" +) + + +def declaration_line_number( + text: str | None, anchor_line: int | None, type_name: str | None = None +) -> int | None: + """Absolute 1-based line of the Java type declaration within chunk ``text``. + + LanceDB chunks are anchored at the chunk's first source line, which for a + file-spanning chunk is the package/import line (``anchor_line`` = 1) while + the ``class``/``interface`` declaration sits several lines down. Without + this, hits render as ``File.java:1`` even though the symbol is declared + later (F8). Returns ``anchor_line + i`` for the first matching declaration + (pinned to ``type_name`` when given, so a nested type doesn't win), or + ``anchor_line`` unchanged when no declaration is found in the chunk. + + Comment-aware: Javadoc/line/block-comment lines that merely MENTION the type + name (e.g. ``* This class Bar handles...``) are skipped so the returned line + is the real declaration, not a comment above it. + """ + if not text or anchor_line is None: + return anchor_line + in_block = False + for i, raw in enumerate(text.splitlines()): + # Drop a trailing ``// ...`` line comment before any matching (a ``//`` + # inside a string literal is unrealistic for a declaration line). + code = raw.split("//", 1)[0] + stripped = code.strip() + if in_block: + if "*/" in stripped: + in_block = False + continue + if stripped.startswith("/*"): + # Single-line ``/* ... */`` -> skip without entering block state. + if "*/" not in stripped[2:]: + in_block = True + continue + if not stripped or stripped.startswith("*"): + # Blank or a Javadoc continuation line (`` * ...``). + continue + m = _JAVA_TYPE_DECL_RE.search(code) + if m and (not type_name or m.group(1) == type_name): + return anchor_line + i + return anchor_line + + def explain_score_components( comps: dict[str, float] | None, *, diff --git a/tests/mcp/test_mcp_v2.py b/tests/mcp/test_mcp_v2.py index fe99412..6f4f5b9 100644 --- a/tests/mcp/test_mcp_v2.py +++ b/tests/mcp/test_mcp_v2.py @@ -281,17 +281,26 @@ def test_find_client_by_target_service(ladybug_graph) -> None: out = find_v2("client", {"target_service": target_service}, graph=ladybug_graph) assert out.success is True assert out.results - assert all(r.fqn.startswith(f"{target_service} ") for r in out.results) + # Client fqn is the contract identifier ``member_fqn->target_service``. Assert + # the full shape (and no Python-None interpolation for member-less clients — + # regression guard for the ``"None->"`` bug). + for r in out.results: + assert "None" not in r.fqn, f"None leaked into fqn: {r.fqn!r}" + if r.member_fqn: + assert r.fqn == f"{r.member_fqn}->{target_service}", r.fqn + else: + assert r.fqn.endswith(target_service) and "->" not in r.fqn, r.fqn def test_find_client_by_path_contains(ladybug_graph) -> None: all_clients = find_v2("client", {}, graph=ladybug_graph) assert all_clients.success is True - sample = next((r for r in all_clients.results if "/" in r.fqn), None) + # Path is carried on the NodeRef ``path`` field (the fqn is the + # ``member_fqn->target_service`` identifier, not a path-bearing string). + sample = next((r for r in all_clients.results if r.path and "/" in r.path), None) if sample is None: pytest.skip("no client rows with path metadata in fixture") - parts = sample.fqn.split(" ") - path = parts[-1] if parts else "" + path = sample.path or "" if not path.startswith("/"): pytest.skip("sample client path is unavailable") needle = path[: min(len(path), 5)] @@ -299,9 +308,7 @@ def test_find_client_by_path_contains(ladybug_graph) -> None: assert out.success is True assert out.results for ref in out.results: - bits = ref.fqn.split(" ") - assert bits - assert needle in bits[-1] + assert needle in (ref.path or "") def test_find_cross_kind_filter_fields_return_failure(ladybug_graph) -> None: diff --git a/tests/package/test_jrag_orientation.py b/tests/package/test_jrag_orientation.py index 0593efd..5d91705 100644 --- a/tests/package/test_jrag_orientation.py +++ b/tests/package/test_jrag_orientation.py @@ -12,7 +12,7 @@ 9. test_search_hybrid_calls_hybrid_path 10. test_search_table_all_runs_three_tables 11. test_search_offset_paginates -12. test_search_fuzzy_rejected_in_handler_as_status_error +12. test_search_fuzzy_accepted_as_silent_noop 13. test_next_actions_valid_runnable_commands_capped_at_5 14. test_next_actions_zero_direction_suppressed 15. test_next_actions_covers_composed_dot_keys @@ -337,22 +337,23 @@ def mock_search_v2(query, **kwargs): ) -def test_search_fuzzy_rejected_in_handler_as_status_error( +def test_search_fuzzy_accepted_as_silent_noop( corpus_root: Path, ladybug_db_path: Path ) -> None: - """--fuzzy is rejected IN-HANDLER with status: error (not argparse exit 2). - - The flag is registered on the parser so argparse doesn't exit 2 before the - handler runs. The handler checks args.fuzzy and produces a canonical error - envelope with the message "search is semantic; --fuzzy is implicit". + """--fuzzy is accepted as a silent no-op (search is always semantic). + + Previously --fuzzy was rejected IN-HANDLER with a canonical + "search is semantic; --fuzzy is implicit" error. Now the flag is ignored + and search proceeds normally. This graph-only fixture has no vector table, + so search surfaces an unrelated missing-table error — the point of the + assertion is that the error is NOT the fuzzy rejection (no "fuzzy" in the + message), proving --fuzzy raised no error of its own. """ env = _env_for(corpus_root, ladybug_db_path) proc = _run_jrag(["search", "test", "--fuzzy", "--format", "json"], env=env) payload = json.loads(proc.stdout) - assert payload["status"] == "error" - msg = payload.get("message") or "" - assert "fuzzy" in msg.lower(), f"expected fuzzy in error message: {msg!r}" - assert "semantic" in msg.lower(), f"expected 'semantic' in message: {msg!r}" + msg = (payload.get("message") or "").lower() + assert "fuzzy" not in msg, f"--fuzzy was rejected: {payload}" # ===== Phase 3 regressions: search score floor + file path (T5-rem) ===== diff --git a/tests/search/test_search_lancedb.py b/tests/search/test_search_lancedb.py index aea1c0f..cf889cd 100644 --- a/tests/search/test_search_lancedb.py +++ b/tests/search/test_search_lancedb.py @@ -126,7 +126,10 @@ def test_vector_displayed_score_is_rank_monotonic() -> None: The honest score uses the adjusted distance (distance + import_penalty - role_weight - symbol_bonus). This matches the sort key, so the displayed score is monotonic. After clamping, scores are in [0,1]. """ - from java_codebase_rag.search.search_lancedb import _effective_distance, l2_distance_to_score, _clamp01 + from java_codebase_rag.search.search_lancedb import ( + _effective_distance, + vector_display_score, + ) # Build controlled rows with varying distances and bonuses rows = [ @@ -167,11 +170,12 @@ def test_vector_displayed_score_is_rank_monotonic() -> None: }, ] - # Simulate the post-sort honest-score pass + # Simulate the post-sort honest-score pass (run_search uses vector_display_score + # on the effective distance — the new non-clamping map; see F6). for r in rows: comps = r["_score_components"] effective_dist = _effective_distance(comps) - r["_score"] = _clamp01(l2_distance_to_score(effective_dist)) + r["_score"] = vector_display_score(effective_dist) # Verify scores are in [0,1] for r in rows: @@ -556,3 +560,97 @@ def test_run_search_dedup_off_is_byte_identical() -> None: # Verify no _chunks_collapsed added for r in result: assert "_chunks_collapsed" not in r, "Should not add _chunks_collapsed when dedup is OFF" + + +# ---------- F7: _silence_lance_autoproj_warnings ---------- + + +def test_silence_lance_warnings_drops_markers_keeps_rest() -> None: + """The autoprojection deprecation lines are swallowed; real stderr survives. + + The silencer redirects the OS-level fd 2 (what LanceDB's Rust tracing writes + to), so the test must write via ``os.write(2, ...)`` — not ``sys.stderr`` + (a Python object that may not be fd 2).""" + import io + import os + import sys + + from java_codebase_rag.search.search_lancedb import _silence_lance_autoproj_warnings + + # Capture what the silencer RE-EMITS to sys.stderr after restoring fd 2. + buf = io.StringIO() + real_stderr = sys.stderr + sys.stderr = buf + try: + with _silence_lance_autoproj_warnings(): + os.write(2, b"WARN tracing: did not include `_distance`. Call disable_scoring_autoprojection\n") + os.write(2, b"ERROR something went wrong: boom\n") + finally: + sys.stderr = real_stderr + emitted = buf.getvalue() + assert "did not include" not in emitted + assert "disable_scoring_autoprojection" not in emitted + assert "ERROR something went wrong: boom" in emitted + + +def test_silence_lance_warnings_opt_out_passthrough(monkeypatch) -> None: + """JAVA_CODEBASE_RAG_KEEP_LANCE_WARNINGS disables the silencer entirely.""" + import sys + + from java_codebase_rag.search.search_lancedb import _silence_lance_autoproj_warnings + + monkeypatch.setenv("JAVA_CODEBASE_RAG_KEEP_LANCE_WARNINGS", "1") + # No redirect happens -> fd 2 is untouched; just verify the context yields. + with _silence_lance_autoproj_warnings(): + sys.stderr.write("") # would land on real stderr (no capture buffer) + # No assertion crash == fd 2 was never hijacked. + + +def test_silence_lance_warnings_restores_fd2_on_exception() -> None: + """fd 2 is restored even when the wrapped call raises.""" + import os + + from java_codebase_rag.search.search_lancedb import _silence_lance_autoproj_warnings + + saved_before = os.dup(2) + try: + with pytest.raises(RuntimeError, match="boom"): + with _silence_lance_autoproj_warnings(): + raise RuntimeError("boom") + # fd 2 must be a valid, open descriptor after the exception. + assert os.fstat(2) is not None + finally: + os.close(saved_before) + + +# ---------- F8: _refine_java_start_lines ---------- + + +def test_refine_java_start_lines_points_at_declaration() -> None: + from java_codebase_rag.search.search_lancedb import _refine_java_start_lines + + chunk = "package com.x;\nimport java.util.List;\n\npublic class ChatPort {\n" + rows = [ + {"_kind": "java", "start": {"line": 1}, "text": chunk, + "_hints": {"primary_type_hint": "ChatPort"}}, + ] + _refine_java_start_lines(rows) + assert rows[0]["start"]["line"] == 4 # decl line, not the package anchor + + +def test_refine_java_start_lines_skips_nonjava_and_method_chunks() -> None: + from java_codebase_rag.search.search_lancedb import _refine_java_start_lines + + rows = [ + # Non-java row untouched. + {"_kind": "sql", "start": {"line": 7}, "text": "SELECT 1", "_hints": {}}, + # Java method-only chunk (no type decl) keeps its anchor. + {"_kind": "java", "start": {"line": 30}, "text": " void doWork() {}\n", + "_hints": {"primary_type_hint": "Worker"}}, + # Missing start dict -> safe (unchanged). + {"_kind": "java", "text": "public class X {", "_hints": {}}, + ] + _refine_java_start_lines(rows) + assert rows[0]["start"]["line"] == 7 + assert rows[1]["start"]["line"] == 30 + assert "start" not in rows[2] diff --git a/tests/search/test_search_scoring.py b/tests/search/test_search_scoring.py new file mode 100644 index 0000000..9d865d2 --- /dev/null +++ b/tests/search/test_search_scoring.py @@ -0,0 +1,85 @@ +"""Unit tests for the pure scoring helpers in ``search_scoring``. + +These functions are dependency-free (no lancedb/torch), so this file runs on +every install — including graph-only (macOS Intel), where the +``search_lancedb`` test module is skipped. +""" + +from __future__ import annotations + +import pytest + +from java_codebase_rag.search.search_scoring import ( + declaration_line_number, + vector_display_score, +) + + +# ---------- vector_display_score (F6) ---------- + + +def test_vector_display_score_is_bounded_and_decreasing() -> None: + # Boundaries of the unit-embedding L2 range [0, 2]. + assert vector_display_score(0.0) == pytest.approx(1.0) + assert vector_display_score(2.0) == pytest.approx(0.0) + # Strictly decreasing: lower distance -> higher score (rank-monotonic). + assert vector_display_score(0.2) > vector_display_score(0.5) > vector_display_score(1.0) + + +def test_vector_display_score_does_not_collapse_past_sqrt2() -> None: + """The whole point of F6: the old ``1 - d²/2`` map clamps to 0 past √2≈1.414, + so a weak-but-best match (d≈1.5) showed score=0.000. The new linear map keeps + it visibly non-zero.""" + assert vector_display_score(1.5) == pytest.approx(0.25) + assert vector_display_score(1.414) == pytest.approx(0.293, abs=1e-3) + assert vector_display_score(1.5) > 0.0 + + +def test_vector_display_score_clamps_to_unit_interval() -> None: + # vector_display_score(d) = clamp01(1 - d/2); nonsensical inputs stay in [0,1]. + assert vector_display_score(3.0) == 0.0 # 1 - 1.5 = -0.5 -> clamps to 0 + assert vector_display_score(-0.5) == 1.0 # 1 + 0.25 = 1.25 -> clamps to 1 + for d in (-1.0, 0.0, 0.7, 1.5, 2.0, 5.0): + assert 0.0 <= vector_display_score(d) <= 1.0 + + +# ---------- declaration_line_number (F8) ---------- + + +def test_declaration_line_number_finds_primary_type() -> None: + chunk = "package com.x;\nimport java.util.List;\n\npublic class ChatPort {\n}\n" + # decl on 1-based line 4 (0-based index 3) + assert declaration_line_number(chunk, anchor_line=1) == 4 + assert declaration_line_number(chunk, anchor_line=10) == 13 # anchor + offset + + +def test_declaration_line_number_skips_javadoc_mentioning_type() -> None: + """Regression for the F8 review finding: a Javadoc line that mentions the + type name (``* This class Bar handles things.``) must NOT win — the returned + line is the real ``public class`` declaration.""" + chunk = "/**\n * This class Bar handles things.\n */\npublic class Bar {\n" + assert declaration_line_number(chunk, anchor_line=1, type_name="Bar") == 4 + + +def test_declaration_line_number_skips_line_and_block_comments() -> None: + line_cmt = "// class Foo is not it\npublic class Foo {\n" + assert declaration_line_number(line_cmt, 1, "Foo") == 2 + block_cmt = "/* stale class Baz\n */\npublic class Baz {\n" + assert declaration_line_number(block_cmt, 1, "Baz") == 3 + + +def test_declaration_line_number_pins_to_named_type_over_nested() -> None: + """When ``type_name`` is given, a nested/earlier type decl must not win.""" + chunk = "class Helper {\n}\npublic class Primary {\n" + assert declaration_line_number(chunk, 1, type_name="Primary") == 3 + # Without a pin, the first decl (Helper) wins. + assert declaration_line_number(chunk, 1) == 1 + + +def test_declaration_line_number_method_only_chunk_keeps_anchor() -> None: + """A method-only chunk (no type decl) returns the anchor unchanged.""" + chunk = " public void doWork() {\n return;\n }\n" + assert declaration_line_number(chunk, anchor_line=42) == 42 + # Empty / missing inputs are safe. + assert declaration_line_number(None, 5) == 5 + assert declaration_line_number("public class X {", None) is None