fix(jrag): address fresh-review findings A-G (silent-wrong-output + test debt)

Folded review fan-out (review-fresh-1..4) findings into one commit.

Code fixes (silent-wrong-output / spec-compliance):
- A: resolve_query returned `ambiguous` with empty candidates when a
  post-filter rejected every `many` candidate; now `not_found` with the
  filter-failure message (an empty ambiguous list had no narrowing value).
- B: _cmd_connection wrapped list_by_capability(MESSAGE_LISTENER) in a bare
  `except: listener_hits = []` -> silent wrong-results; now warns-and-continues
  so an empty async inbound section is distinguishable from "no listeners".
- C: find query-mode truncation was decided on the POST-filtered list, so a
  post-filter that dropped rows silently cleared `truncated`; now decided on
  the raw name/FQN fetch (limit+1), with a warning when post-filters apply
  after a capped fetch. find filter-mode didn't slice out.results -> displayed
  limit+1 rows at the boundary; now slices to limit and sets truncated from
  has_more_results OR len>limit.
- D: overrides/overridden-by set direction="up"/"down" on edge rows, tripping
  the renderer's has_direction guard -> mislabeled flat lists as
  `↑ supertypes:` / `↓ subtypes:`. Dropped the direction key (flat is correct).
- E: jrag_hints result_edges fallback emitted self-hints (after `callers` it
  suggested `jrag callers` again). Added current_command param (plumbed via
  next_actions_hook + _emit_traversal) to skip the self-hint; the inverse
  direction (`callees` after `callers`) is kept — that's the useful signal.
- F: warnings[] were JSON-only (the renderer never emitted them in text), so
  the "inapplicable flags never silently ignored" spec was unenforced for text
  consumers. The renderer now appends `warning:` lines; status/microservices/
  map/conventions/topics/impact now warn on inherited flags they don't apply.

Tests (G + pinning):
- Pin A/D/E/F with focused regression tests.
- Strengthen vacuous assertions: test_resolve_service (7/10 skipped -> use the
  ladybug_db_path fixture; tautological `status in (one,many,none)` -> per-branch
  contracts), test_jrag_listing (truncated now actually verified; client_kind
  enum must not accept un-normalized "feign"; routes must have `path`),
  test_jrag_locate (find_by_capability/annotation prove narrowing vs.
  unfiltered; inspect-ambiguous asserts each branch instead of `elif ok: pass`),
  test_jrag_orientation test 7 (overview --as vacuous `if status==ok` guard ->
  unconditional dispatch assertion).

Verified: 138 focused jrag/resolve tests green; full suite 1006 passed / 16
failed, all 16 pre-existing or fixture-pollution (4 TestPR4IndexProgress are
order-dependent pollution from test_java_codebase_rag_cli init/erase tests
running against the real corpus_root — tracked as a separate follow-up, not a
regression from this commit).

Co-Authored-By: Claude <noreply@anthropic.com>

Author: HumanBean17

java_codebase_rag/jrag.py

@@ -882,6 +882,7 @@ def _cmd_status(args: argparse.Namespace) -> int:
     # ``counts`` / ``edges`` render as indented alphabetical sections without
     # abusing ``edge_summary`` (which is reserved for PR-JRAG-3 real edge
     # data). See jrag_render._render_inspect / _render_text_shape.
+    warnings = _warn_inapplicable_common(args, service=True, module=True, limit=True)
     env = Envelope(
         status="ok",
         nodes={
@@ -897,6 +898,7 @@ def _cmd_status(args: argparse.Namespace) -> int:
                 "edges": dict(edge_counts),
             },
         },
+        warnings=warnings,
     )
     print(render(env, fmt=args.format, noun="status", shape="inspect"))
     return 0
@@ -1015,7 +1017,7 @@ def _cmd_find_query_mode(
     ``_cmd_find``, so the only ``kinds`` filter we may pass is symbol sub-kinds
     derived from ``--java-kind``.
     """
-    from java_codebase_rag.jrag_envelope import Envelope, mark_truncated, next_actions_hook, normalize_enum
+    from java_codebase_rag.jrag_envelope import Envelope, next_actions_hook, normalize_enum
     from java_codebase_rag.jrag_render import render
 
     query = args.query
@@ -1037,17 +1039,27 @@ def _cmd_find_query_mode(
         microservice=args.service,
         limit=limit + 1,  # +1 for truncated detection
     )
+    # Truncation is decided by the RAW name/FQN fetch (limit+1), BEFORE
+    # post-filters reduce the set — otherwise a post-filter that drops rows
+    # would silently clear `truncated` even though more name matches may exist
+    # beyond the fetch (silent wrong-results).
+    raw_truncated = len(rows) > limit
 
     # Post-filter by role/annotation/capability (SymbolHit carries these).
+    post_filter_active = False
     if args.role:
+        post_filter_active = True
         role_norm = normalize_enum(args.role, kind="role")
         rows = [r for r in rows if (r.role or "").upper().replace("-", "_") == role_norm.upper()]
     if args.exclude_role:
+        post_filter_active = True
         exclude_role_norm = normalize_enum(args.exclude_role, kind="role")
         rows = [r for r in rows if (r.role or "").upper().replace("-", "_") != exclude_role_norm.upper()]
     if args.annotation:
+        post_filter_active = True
         rows = [r for r in rows if args.annotation in (r.annotations or [])]
     if args.capability:
+        post_filter_active = True
         rows = [r for r in rows if args.capability in (r.capabilities or [])]
 
     # Build warnings for filters that cannot apply in query mode. SymbolHit
@@ -1062,10 +1074,19 @@ def _cmd_find_query_mode(
         warnings.append(
             "--source-layer ignored in query mode (applies to routes; use filter mode)"
         )
+    # When post-filters apply after a capped fetch, `truncated` reflects the
+    # pre-filter name-match count and cannot know whether MORE filtered matches
+    # exist beyond the fetch — surface that honestly.
+    if raw_truncated and post_filter_active:
+        warnings.append(
+            "results truncated before --role/--annotation/--capability filters; "
+            "additional filtered matches may exist beyond the fetch"
+        )
 
-    # Convert SymbolHit rows to NodeRef-like dicts for the envelope.
+    # Display at most `limit` of the (post-filtered) rows.
+    display_rows = rows[:limit]
     nodes = {}
-    for row in rows:
+    for row in display_rows:
         node_id = row.id
         nodes[node_id] = {
             "id": node_id,
@@ -1078,15 +1099,7 @@ def _cmd_find_query_mode(
             "role": row.role,
         }
 
-    # mark_truncated operates on a list; envelope.nodes is a dict keyed by id.
-    # Round-trip dict -> list -> truncate -> dict to apply the +1-fetch drop
-    # (the truncated flag is computed off the list length, which equals the
-    # dict size, so this is sound).
-    node_list = list(nodes.values())
-    display_nodes_list, truncated = mark_truncated(node_list, limit)
-    display_nodes = {node["id"]: node for node in display_nodes_list}
-
-    env = Envelope(status="ok", nodes=display_nodes, truncated=truncated, warnings=warnings)
+    env = Envelope(status="ok", nodes=nodes, truncated=raw_truncated, warnings=warnings)
     next_actions_hook(env)
 
     # Offset is not supported in query mode (find_by_name_or_fqn has no offset).
@@ -1162,9 +1175,14 @@ def _cmd_find_filter_mode(
         print(render(env, fmt=args.format))
         return 2
 
-    # Convert results to envelope rows
-    nodes_dict = {ref.id: to_envelope_rows([ref])[0] for ref in out.results}
-    truncated = out.has_more_results or False
+    # Convert results to envelope rows. Slice to `limit`: find_v2 was called with
+    # limit+1, so when exactly user_limit+1 matches exist `out.results` carries
+    # one extra row that must be dropped (off-by-one guard). `truncated` is True
+    # when the backend reports more OR the +1 row is present.
+    results = list(out.results)
+    truncated = bool(out.has_more_results) or len(results) > limit
+    display_refs = results[:limit]
+    nodes_dict = {ref.id: to_envelope_rows([ref])[0] for ref in display_refs}
 
     env = Envelope(status="ok", nodes=nodes_dict, truncated=truncated)
     next_actions_hook(env)
@@ -1331,6 +1349,13 @@ def _cmd_topics(args: argparse.Namespace) -> int:
         warnings.append(
             f"{no_topic_count} producer(s) had no topic and were excluded"
         )
+    # list_producers has no module kwarg (only microservice/topic_prefix); --module
+    # would be silently dropped — surface it (use --producer-in to scope by svc).
+    if getattr(args, "module", None):
+        warnings.append(
+            "--module is not applied on topics (list_producers has no module param; "
+            "use --producer-in to scope producers by microservice)"
+        )
 
     # If --consumer-in is provided, resolve consumers for each topic group.
     # A consumer of a topic IS a listener: the edge path is
@@ -1584,7 +1609,7 @@ def _emit_traversal(
         warnings=warnings or [],
         truncated=truncated,
     )
-    next_actions_hook(env, root=root_id, result_edges=edges)
+    next_actions_hook(env, root=root_id, result_edges=edges, command=getattr(args, "command", None))
     print(render(env, fmt=args.format, noun=noun))
     return 0
 
@@ -1635,6 +1660,30 @@ def _warn_unapplied_scope(args: argparse.Namespace, *, reason: str) -> list[str]
     return warnings
 
 
+def _warn_inapplicable_common(
+    args: argparse.Namespace, *, service: bool, module: bool, limit: bool
+) -> list[str]:
+    """Warn when common flags that don't apply to a command are set.
+
+    Companion to :func:`_warn_unapplied_scope` for the aggregate / orientation
+    commands (status / microservices / map / conventions) which inherit the
+    ``common`` parent parser (``--service`` / ``--module`` / ``--limit``) but
+    don't apply all of them. Each kwarg names whether THAT flag is inapplicable
+    for this command (``True`` -> warn if the user set it). The plan principle
+    "inapplicable flags never silently ignored" requires the warning; with the
+    renderer now printing ``warning:`` lines, this is visible to text consumers
+    too (not just ``--format json``).
+    """
+    warnings: list[str] = []
+    if service and args.service:
+        warnings.append("--service is not applied on this command")
+    if module and getattr(args, "module", None):
+        warnings.append("--module is not applied on this command")
+    if limit and getattr(args, "limit", None) is not None and args.limit != 20:
+        warnings.append("--limit is not applied on this command")
+    return warnings
+
+
 def _cmd_callers(args: argparse.Namespace) -> int:
     cfg, graph, rc = _load_graph_or_error(args)
     if rc:
@@ -2013,7 +2062,10 @@ def _cmd_overrides(args: argparse.Namespace) -> int:
     edges: list[dict] = []
     for e in out.results:
         nodes[e.other.id] = _noderef_to_node_dict(e.other)
-        edges.append({"other_id": e.other.id, "edge_type": "OVERRIDES", "direction": "up"})
+        # No `direction` key: overrides is a flat list, not a tree. Setting
+        # direction="up" would trip the renderer's has_direction guard and
+        # mis-label these rows as `↑ supertypes:` (hierarchy). Flat is correct.
+        edges.append({"other_id": e.other.id, "edge_type": "OVERRIDES"})
     truncated = bool(out.has_more_results) or len(edges) > limit
     if len(edges) > limit:
         edges = edges[:limit]
@@ -2062,7 +2114,10 @@ def _cmd_overridden_by(args: argparse.Namespace) -> int:
     edges: list[dict] = []
     for e in out.results:
         nodes[e.other.id] = _noderef_to_node_dict(e.other)
-        edges.append({"other_id": e.other.id, "edge_type": "OVERRIDES", "direction": "down"})
+        # No `direction` key — see _cmd_overrides: a `direction` value would
+        # route these into the hierarchy renderer (`↓ subtypes:`), mis-labeling
+        # a flat overridden-by list.
+        edges.append({"other_id": e.other.id, "edge_type": "OVERRIDES"})
     truncated = bool(out.has_more_results) or len(edges) > limit
     if len(edges) > limit:
         edges = edges[:limit]
@@ -2137,6 +2192,11 @@ def _cmd_impact(args: argparse.Namespace) -> int:
             "--service is a post-filter on impact (impact_analysis has no microservice param)"
         )
         impacts = [h for h in impacts if (h.microservice or "") == args.service]
+    if getattr(args, "module", None):
+        # impact_analysis has no module param either; warn rather than drop silently.
+        warnings.append(
+            "--module is not applied on impact (impact_analysis has no module param)"
+        )
     display, truncated = mark_truncated(impacts, limit)
     root_id = node.id
     nodes: dict[str, dict] = {root_id: _noderef_to_node_dict(node)}
@@ -2487,7 +2547,13 @@ def _calls_service_match_in(caller_microservice: str) -> bool:
                 microservice=microservice,
                 limit=_CONSUMER_FETCH_LIMIT,
             )
-        except Exception:
+        except Exception as e:  # noqa: BLE001 - best-effort multi-section view
+            # Don't swallow silently: surface the failure so an empty async
+            # inbound section is distinguishable from "no listeners". HTTP
+            # inbound above is unaffected; the command still returns its other
+            # sections. (The bare `except: listener_hits = []` this replaces
+            # produced silent wrong-results — status:ok with no async + no clue.)
+            warnings.append(f"listener lookup failed; async inbound section skipped: {e}")
             listener_hits = []
         topic_route_ids: set[str] = set()
         for h in listener_hits:
@@ -2808,9 +2874,11 @@ def _cmd_microservices(args: argparse.Namespace) -> int:
         return rc
 
     counts = graph.microservice_counts()
+    warnings = _warn_inapplicable_common(args, service=True, module=True, limit=True)
     env = Envelope(
         status="ok",
         nodes={"microservices": {"counts": dict(counts)}},
+        warnings=warnings,
     )
     next_actions_hook(env)
     print(render(env, fmt=args.format, noun="microservices", shape="inspect"))
@@ -2857,9 +2925,13 @@ def _cmd_map(args: argparse.Namespace) -> int:
         kind = str(r.get("kind") or "(unknown)")
         grouped.setdefault(scope, {})[kind] = int(r.get("n") or 0)
 
+    # --service/--module are applied above (scope_clauses); --limit is not (this
+    # is an aggregate count, not a row fetch).
+    warnings = _warn_inapplicable_common(args, service=False, module=False, limit=True)
     env = Envelope(
         status="ok",
         nodes={"map": {"group_by": group_col, "counts": grouped}},
+        warnings=warnings,
     )
     next_actions_hook(env)
     print(render(env, fmt=args.format, noun="map", shape="inspect"))
@@ -2905,9 +2977,13 @@ def _cmd_conventions(args: argparse.Namespace) -> int:
         if fw:
             framework_counts[fw] = int(r.get("n") or 0)
 
+    # --service is applied above; --module/--limit are not (no module clause;
+    # aggregate count).
+    warnings = _warn_inapplicable_common(args, service=False, module=True, limit=True)
     env = Envelope(
         status="ok",
         nodes={"conventions": {"roles": role_counts, "frameworks": framework_counts}},
+        warnings=warnings,
     )
     next_actions_hook(env)
     print(render(env, fmt=args.format, noun="conventions", shape="inspect"))

java_codebase_rag/jrag_envelope.py

@@ -351,6 +351,18 @@ def resolve_query(
             if loc is not None:
                 env.file_location = loc
             return node, env
+        if not survivors:
+            # Every `many` candidate was rejected by the post-filters — there is
+            # nothing left to disambiguate, so this is not_found, NOT an empty
+            # ambiguous list (which would render as "0 ambiguous matches" with no
+            # narrowing value). Same message as the `one` post-filter-fail branch.
+            return None, Envelope(
+                status="not_found",
+                message=(
+                    f"No matches for {identifier!r} after applying --java-kind/--role/--fqn-prefix "
+                    "filters; use `jrag search <query>` for ranked fuzzy lookup."
+                ),
+            )
         capped = survivors[:10]
         env = Envelope(
             status="ambiguous",
@@ -373,6 +385,7 @@ def next_actions_hook(
     root: str | None = None,
     edge_summary: dict[str, Any] | None = None,
     result_edges: list[dict[str, Any]] | None = None,
+    command: str | None = None,
 ) -> list[str]:
     """Populate ``envelope.agent_next_actions`` via :mod:`jrag_hints` (PR-JRAG-4).
 
@@ -423,5 +436,6 @@ def next_actions_hook(
         root_fqn=root_fqn,
         edge_summary=edge_summary,
         result_edges=result_edges if result_edges is not None else list(envelope.edges),
+        current_command=command,
     )
     return envelope.agent_next_actions

java_codebase_rag/jrag_hints.py

@@ -82,6 +82,7 @@ def next_actions(
     edge_summary: dict[str, Any] | None = None,
     result_edges: list[dict[str, Any]],
     graph: Any = None,  # noqa: ARG001 — reserved for future use (brief contract)
+    current_command: str | None = None,
 ) -> list[str]:
     """Build ``agent_next_actions`` hints for a resolved root.
 
@@ -142,7 +143,11 @@ def _add(cmd: str) -> None:
                 _add(cmds["out"])
     else:
         # traversal path: infer from result_edges labels.
-        # No per-direction counts → emit both directions for recognized labels.
+        # No per-direction counts → emit both directions for recognized labels,
+        # then drop the self-hint (the command an agent just ran). The inverse
+        # direction (e.g. `callees` after `callers`) is the useful exploration
+        # signal and is kept; only the exact command just run is redundant.
+        # ``current_command`` is the jrag subcommand name (``args.command``).
         labels_seen: set[str] = set()
         for edge in result_edges or []:
             et = str(edge.get("edge_type") or "").strip()
@@ -152,9 +157,8 @@ def _add(cmd: str) -> None:
             cmds = _LABEL_COMMANDS.get(label)
             if cmds is None:
                 continue
-            if "in" in cmds:
-                _add(cmds["in"])
-            if "out" in cmds:
-                _add(cmds["out"])
+            for d in ("in", "out"):
+                if d in cmds and cmds[d] != current_command:
+                    _add(cmds[d])
 
     return hints[:_MAX_HINTS]

java_codebase_rag/jrag_render.py

@@ -404,4 +404,12 @@ def render(
     if envelope.truncated:
         hint = _truncated_hint(next_offset=next_offset)
         body = f"{body}\n{hint}" if body else hint
+    # Warnings are rendered in text mode (one ``warning:`` line each) so an
+    # agent running without ``--format json`` still sees inapplicable-flag /
+    # post-filter notices. Without this the warnings[] field was JSON-only and
+    # the "inapplicable flags never silently ignored" spec was effectively
+    # unenforced for text consumers.
+    if envelope.warnings:
+        warning_lines = "\n".join(f"warning: {w}" for w in envelope.warnings)
+        body = f"{body}\n{warning_lines}" if body else warning_lines
     return body

tests/test_jrag_envelope.py

@@ -269,6 +269,48 @@ def test_resolve_query_many_caps_candidates_at_ten(monkeypatch: pytest.MonkeyPat
     assert len(env.candidates) == 10  # capped
 
 
+def test_resolve_query_many_post_filter_rejects_all_is_not_found(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Regression (review finding A): when a post-filter rejects EVERY `many`
+    candidate, the result is not_found — NOT an empty ambiguous list.
+
+    An empty ambiguous list would render as '0 ambiguous matches' with no
+    narrowing value; not_found with the filter-failure message is the honest,
+    actionable result (same message as the `one` post-filter-fail branch).
+    """
+    n1 = _make_node(id="sym:1", fqn="com.foo.Bar.doStuff", microservice="foo", role="SERVICE")
+    n2 = _make_node(id="sym:2", fqn="com.foo.Baz.doStuff", microservice="bar", role="SERVICE")
+    fake_output = ResolveOutput(
+        success=True,
+        status="many",
+        candidates=[
+            ResolveCandidate(node=n1, score=0.9, reason="fqn_suffix"),
+            ResolveCandidate(node=n2, score=0.5, reason="short_name"),
+        ],
+    )
+    monkeypatch.setattr("resolve_service.resolve_v2", lambda *a, **kw: fake_output)
+    graph = MagicMock()
+    cfg = MagicMock()
+
+    result_node, env = resolve_query(
+        "doStuff",
+        hint_kind="symbol",
+        java_kind=None,
+        role="CONTROLLER",  # neither candidate is CONTROLLER -> all rejected
+        fqn_prefix=None,
+        cfg=cfg,
+        graph=graph,
+    )
+    assert result_node is None
+    assert env.status == "not_found", (
+        f"empty-many must be not_found (not {env.status!r}); candidates={env.candidates}"
+    )
+    assert env.candidates == []
+    assert env.message is not None
+    assert "filters" in env.message.lower() or "post-filter" in env.message.lower()
+
+
 def test_resolve_query_none_is_not_found_with_search_hint(monkeypatch: pytest.MonkeyPatch) -> None:
     fake_output = ResolveOutput(
         success=True,