[Bug] CLI: resolve --mode is accepted, documented, and recommended but never affects retrieval

[AdityaPainuli]

Area

Other

Potpie version

potpie v2.0.0

Install source

uv tool install potpie

System

macOS

Coding agent and model

No response

Command or workflow

# Identical retrieval regardless of mode
potpie --json resolve "how does the payment retry flow work?" --mode fast
potpie --json resolve "how does the payment retry flow work?" --mode deep

# Invalid values are accepted silently and echoed back
potpie --json resolve "some task" --mode garbage

# The MCP agent surface exposes the same inert parameter
# context_resolve(task=..., mode="deep")

What happened?

Problem

potpie resolve --mode (documented as fast | balanced | verify | deep, described as retrieval depth) is accepted and echoed back in the response, but no code path ever reads it.

Observed Behavior

• ResolveRequest.mode and SearchRequest.mode have exactly two consumers in the engine, both metadata echoes: graph_service.py:125 and graph_service.py:139 (metadata={"mode": request.mode, ...}). Nothing downstream — orchestrator, readers, ranker — branches on it. The only retrieval-shaping preference on the read path is freshness_preference.
• ResolveMode = str (domain/ports/agent_context.py:33) with the valid values living only in a comment, so --mode garbage is accepted and reported back as if it were a real mode.
• The inert knob is advertised on three surfaces:
  • the CLI reference (docs/context-graph/cli-flow.md) documents --mode fast|balanced|verify|deep;
  • the MCP agent contract exposes mode: str = "fast" on context_resolve (adapters/inbound/mcp/server.py:106), so harness agents are offered a parameter that does nothing;
  • CONTEXT_RESOLVE_RECIPES assigns per-intent modes (balanced for review/operations/planning/refactor/security) and report_status.py serves them in the recommended recipe — so potpie status actively recommends a mode that resolve then discards.

Excepted Behavior

One of:

• mode actually shapes retrieval (e.g. depth/limits/verification behavior differing between fast and deep), or
• the value is validated against the documented set and the response transparently reports it as not yet implemented, rather than echoing it back as if it were honored.

Either way, an invalid --mode value should be rejected or flagged, not silently accepted.

Impact

Same failure class as #994 and #996: a documented flag that silently does nothing. Agents (and the skill/recipe guidance itself) select modes expecting different retrieval depth, spend nothing differently, and trust an output header that implies the mode was applied. Since the recommended recipes ship non-default modes, every agent following potpie status guidance is passing a dead parameter.

Source or build details

No response

Output or logs

[linear-code]

POT-1940

[coderabbitai]

Coding Plan

Summary

• Choose the "honest, validated, not-yet-implemented" resolution over inventing real per-mode retrieval semantics inside a bug fix.
• Enforce mode validation once at the shared ResolveRequest/SearchRequest boundary (fail-loud ValueError) using a single canonical RESOLVE_MODES set, so both CLI and MCP reject invalid values via existing error boundaries.
• Make responses transparently report mode as accepted-but-inert via envelope metadata, and surface that in CLI output (MCP already echoes metadata).
• Strip overpromising language from cli-flow.md, the MCP contract/help, the agent-contract manifest, and report_status recommendations, and add the missing test coverage.

Design Choices

Design Choice 1: Should `mode` be made to genuinely shape retrieval, or validated-and-flagged as not-yet-implemented?

Options Considered:

1. Implement real mode behavior end-to-end (depth/limit/verification differences per mode), following the freshness_preference threading pattern.
2. Validate mode against the documented set (reject invalid) and have responses transparently report that mode is accepted-but-inert, while stripping overpromising language from docs/contract/recommendations.

Chosen Option: 2

Rationale: The ticket explicitly offers either outcome ("One of…"). Making mode functionally alter depth/limits/verification would require threading a new parameter through ReadOrchestrator, every reader, and RankingService (mirroring freshness_preference) plus defining concrete per-mode retrieval semantics — a large, product-defining effort well beyond a bug fix. Option 2 directly resolves the bug (no silent acceptance, no dishonest echo) with minimal, low-risk changes, aligns with the existing UnsupportedInclude/CapabilityNotImplemented honesty conventions, and leaves genuine per-mode retrieval semantics to a deliberate V2 feature rather than inventing them inside a bug fix.

Design Choice 2: Should invalid mode values be hard-rejected or coerced to a safe default?

Options Considered:

1. Fail-soft coerce unknown modes to "fast" (reuse existing normalize_resolve_mode as-is).
2. Fail-loud: raise ValueError for out-of-set values at the shared request boundary so both CLI and MCP surface a structured rejection.

Chosen Option: 2

Rationale: The orphaned normalize_resolve_mode coerces unknown values to "fast" (fail-soft), but the ticket states an invalid value "should be rejected or flagged, not silently accepted," and coercion is a form of silent acceptance. Coercion would still hide caller error; raising ValueError is caught by the CLI contract() boundary (→ validation_error exit) and the MCP except (ValueError, CapabilityNotImplemented) handler (→ {"ok": false, ...}), giving an explicit rejection on both surfaces with no new plumbing.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Phase 1: Validate mode against a single canonical value set

This phase establishes one authoritative set of allowed modes and enforces it at the shared domain boundary that both the CLI and MCP funnel through, so invalid values are rejected instead of silently accepted and echoed.

Task 1: Establish a canonical mode vocabulary and a fail-loud validator

Consolidate the currently-drifting definitions of the allowed modes into one source of truth and expose a strict validator.

• Treat RESOLVE_MODES in domain/source_references.py ({"fast","balanced","deep","verify"}) as the canonical set, matching the CLI/docs help text.
• Add a strict validation helper alongside the existing fail-soft normalize_resolve_mode (e.g. a validate_resolve_mode that raises ValueError with a clear message listing allowed values) rather than mutating the existing coercing function that tests pin.
• Note and reconcile the drift: tests/unit/test_agent_surface_contract.py's local VALID_MODES omits "verify"; align it with the canonical set as part of Phase 4.

Task 2: Enforce validation at the shared request boundary

Validate mode once, where both inbound adapters converge, so neither surface can pass an unvalidated string downstream.

• Add validation in the frozen dataclasses ResolveRequest and SearchRequest (domain/ports/agent_context.py) via __post_init__, calling the strict validator so any out-of-set value raises ValueError at construction time.
• This causes the CLI (query.py inside with contract():) to emit the documented validation_error exit path and the MCP tool (server.py) to return _error(exc) — both existing boundaries already handle ValueError, so no adapter-level try/except changes are required.
• Keep ResolveMode as the documented type alias but update its comment to point at the canonical set.

🤖 Prompt for AI agents

Implement mode validation at a single canonical source of truth and enforce it
at the shared domain request boundary so invalid `mode` values are rejected
rather than silently accepted.

- Treat `RESOLVE_MODES` in `domain/source_references.py`
(`{"fast","balanced","deep","verify"}`) as the canonical allowed set, matching
CLI/docs help text.
- Add a new strict validation helper (e.g. `validate_resolve_mode`) alongside
the existing fail-soft `normalize_resolve_mode` — it should raise `ValueError`
with a clear message listing allowed values for out-of-set input. Do not mutate
the existing coercing function since tests pin its behavior.
- Enforce this validator in `__post_init__` of the frozen dataclasses
`ResolveRequest` and `SearchRequest` in `domain/ports/agent_context.py`, so any
out-of-set `mode` value raises `ValueError` at construction time.
- Verify this causes the CLI (`query.py`, inside `with contract():`) to emit the
documented `validation_error` exit path, and the MCP tool (`server.py`) to
return `_error(exc)` via its existing `except (ValueError,
CapabilityNotImplemented)` handling — no adapter-level try/except changes should
be needed since both boundaries already handle `ValueError`.
- Keep `ResolveMode` as the documented type alias but update its comment to
reference the canonical set.
- Note (for later reconciliation in Phase 4):
`tests/unit/test_agent_surface_contract.py` has a local `VALID_MODES` that omits
`"verify"` and will need aligning to the canonical set.

Phase 2: Report mode transparently as accepted-but-not-yet-implemented

This phase stops responses from implying the mode was honored. It reuses the existing "honest gap" philosophy (as with UnsupportedInclude) but expresses it for the mode axis through envelope metadata, and ensures the CLI surfaces it instead of silently dropping it.

Task 1: Flag mode as inert in the response envelope

Make the response state plainly that mode did not influence retrieval.

• In DefaultGraphService.resolve and .search (application/services/graph_service.py:125,139), where mode is written into metadata, add an explicit honesty signal alongside it — e.g. mode_applied: False plus a short mode_note explaining the value was accepted but does not yet affect retrieval depth in this version.
• Follow the in-code precedent of the "task accepted but ignored in V1.5" comment: annotate these lines so future readers understand the field is intentionally inert pending real implementation.

Task 2: Surface the mode signal in CLI output

Ensure the CLI no longer silently discards mode from its payload, so potpie --json resolve output reflects the honesty signal like the MCP surface already does.

• Update _envelope_payload (and the human renderer) in adapters/inbound/cli/commands/query.py to surface mode, mode_applied, and the note from env.metadata, so the CLI JSON/human output no longer implies the mode was honored.
• The MCP tool already echoes env.to_dict()["metadata"], so the new fields flow through automatically there; verify the not-implemented flag is present in that payload.

🤖 Prompt for AI agents

Implement transparent reporting so responses no longer imply `mode` influenced
retrieval, and ensure the CLI surfaces this honesty signal.

- In `application/services/graph_service.py`, in `DefaultGraphService.resolve`
and `.search` (around lines 125 and 139), alongside the existing write of `mode`
into `metadata`, add an explicit honesty signal: `mode_applied: False` plus a
short `mode_note` explaining that the value was accepted but does not yet affect
retrieval depth in this version.
- Follow the existing in-code precedent of the "`task` accepted but ignored in
V1.5" comment style, annotating these lines so future readers understand the
field is intentionally inert pending real implementation.
- Update `_envelope_payload` (and the human renderer) in
`adapters/inbound/cli/commands/query.py` to surface `mode`, `mode_applied`, and
the note from `env.metadata`, so CLI JSON/human output no longer implies mode
was honored.
- Confirm the MCP tool (which already echoes `env.to_dict()["metadata"]`)
automatically carries these new fields through, and verify the not-implemented
flag is present in that payload.

Phase 3: Remove overpromising guidance from docs, contract, and status recommendations

This phase ensures the surrounding surfaces stop advertising or recommending mode behavior that does not exist, closing the loop so potpie status and the agent contract no longer steer callers toward a dead parameter.

Task 1: Align the agent contract/recipe guidance with reality

Prevent the manifest and recommendations from promising retrieval escalation that isn't implemented.

• In domain/agent_context_port.py, revise the manifest rule text that promises escalation "to … deep" so it no longer implies mode changes retrieval; make clear mode is advisory/not-yet-honored. Keep the per-intent recipe mode values intact (they remain intent-appropriate guidance for future implementation) to avoid breaking the curated-recipe contract tests.
• In application/use_cases/report_status.py, annotate the recommended recipe / recommended_next_actions (the resolve action) to carry the honesty note that mode is not yet applied, so potpie status no longer recommends a mode as if it were effective.

Task 2: Correct user- and agent-facing documentation

Make the documented behavior honest on every surface that advertises the flag.

• Update docs/context-graph/cli-flow.md to state clearly that --mode is validated against fast|balanced|verify|deep but does not yet change the read path (strengthen the existing V1.5 caveat and align the version wording).
• Update the context_resolve tool docstring/parameter description in adapters/inbound/mcp/server.py and the --mode help text in query.py to reflect that the value is validated but not yet honored.

🤖 Prompt for AI agents

Update the surrounding surfaces (agent contract manifest, status
recommendations, and documentation) so none of them advertise or recommend
`mode` behavior that isn't implemented.

- In `domain/agent_context_port.py`, revise the manifest rule text that
currently promises escalation "to … deep" so it no longer implies mode changes
retrieval; clarify that mode is advisory/not-yet-honored. Keep the per-intent
recipe `mode` values in `CONTEXT_RESOLVE_RECIPES` unchanged, since they remain
intent-appropriate guidance for future implementation and are pinned by
curated-recipe contract tests.
- In `application/use_cases/report_status.py`, annotate the recommended recipe /
`recommended_next_actions` (specifically the `resolve` action) to carry the
honesty note that mode is not yet applied, so `potpie status` no longer
recommends a mode as if it were effective.
- Update `docs/context-graph/cli-flow.md` to clearly state that `--mode` is
validated against `fast|balanced|verify|deep` but does not yet change the read
path — strengthen the existing V1.5 caveat and align the version wording.
- Update the `context_resolve` tool docstring/parameter description in
`adapters/inbound/mcp/server.py`, and the `--mode` help text in `query.py`, to
reflect that the value is validated but not yet honored.

Phase 4: Tests

No tests currently pin --mode behavior for either surface; add coverage for the new validation and honesty signaling and reconcile the drifting mode set.

Task 1: Add validation and flagging tests

Cover the new reject/flag behavior at each boundary.

• Add unit tests for the strict validator (raises on unknown, accepts each canonical value) near tests/unit/test_source_references.py.
• Add tests asserting ResolveRequest/SearchRequest construction rejects an invalid mode (ValueError).
• Add CLI/MCP-level tests asserting an invalid --mode/mode is rejected (CLI validation_error path; MCP {"ok": False, "error": "ValueError"}), and that a valid mode returns a response containing the mode_applied: False/not-implemented signal.

Task 2: Reconcile existing contract tests

Bring the drifting expectations in line with the canonical set.

• Update VALID_MODES in tests/unit/test_agent_surface_contract.py to match the canonical RESOLVE_MODES (include "verify"), and confirm test_agent_context_port.py recipe assertions still hold since recipe mode values are left unchanged.
• Adjust any report_status/manifest tests affected by the added honesty note.

🤖 Prompt for AI agents

Add test coverage for the new validation and honesty-signaling behavior, and
reconcile existing tests that had drifted from the canonical mode set.

- Add unit tests for the strict validator (raises `ValueError` on unknown
values, accepts each canonical value) near
`tests/unit/test_source_references.py`.
- Add tests asserting that `ResolveRequest`/`SearchRequest` construction rejects
an invalid `mode` with `ValueError`.
- Add CLI/MCP-level tests asserting an invalid `--mode`/`mode` is rejected: CLI
should follow the `validation_error` exit path; MCP should return `{"ok": False,
"error": "ValueError"}`. Also assert that a valid mode returns a response
containing the `mode_applied: False`/not-implemented signal.
- Update `VALID_MODES` in `tests/unit/test_agent_surface_contract.py` to match
the canonical `RESOLVE_MODES` set (including `"verify"`), and confirm
`test_agent_context_port.py` recipe assertions still hold since recipe `mode`
values are left unchanged.
- Adjust any `report_status`/manifest tests affected by the added honesty note.

Research

potpie/context-engine/ uses a hexagonal architecture: inbound adapters (CLI in adapters/inbound/cli/commands/query.py, MCP in adapters/inbound/mcp/server.py) funnel into shared domain request DTOs (domain/ports/agent_context.py: ResolveRequest/SearchRequest, ResolveMode = str), which are executed by application/services/graph_service.py (DefaultGraphService.resolve/search) over a ReadOrchestrator.

The read path already threads freshness_preference as a real retrieval-shaping parameter (through orchestrator → readers → RankingService), providing the reference pattern for a "real" knob. In contrast, mode is written only into AgentEnvelope.metadata at graph_service.py:125 and :139 and never read again.

An orphaned validator (normalize_resolve_mode/RESOLVE_MODES in domain/source_references.py) and an honest-gap convention for the include axis (UnsupportedInclude/CapabilityNotImplemented) already exist and can be leveraged.

Per-intent recommended modes live in CONTEXT_RESOLVE_RECIPES (domain/agent_context_port.py) and are surfaced by application/use_cases/report_status.py.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Implement mode validation at a single canonical source of truth and enforce it
at the shared domain request boundary so invalid `mode` values are rejected
rather than silently accepted.

- Treat `RESOLVE_MODES` in `domain/source_references.py`
(`{"fast","balanced","deep","verify"}`) as the canonical allowed set, matching
CLI/docs help text.
- Add a new strict validation helper (e.g. `validate_resolve_mode`) alongside
the existing fail-soft `normalize_resolve_mode` — it should raise `ValueError`
with a clear message listing allowed values for out-of-set input. Do not mutate
the existing coercing function since tests pin its behavior.
- Enforce this validator in `__post_init__` of the frozen dataclasses
`ResolveRequest` and `SearchRequest` in `domain/ports/agent_context.py`, so any
out-of-set `mode` value raises `ValueError` at construction time.
- Verify this causes the CLI (`query.py`, inside `with contract():`) to emit the
documented `validation_error` exit path, and the MCP tool (`server.py`) to
return `_error(exc)` via its existing `except (ValueError,
CapabilityNotImplemented)` handling — no adapter-level try/except changes should
be needed since both boundaries already handle `ValueError`.
- Keep `ResolveMode` as the documented type alias but update its comment to
reference the canonical set.
- Note (for later reconciliation in Phase 4):
`tests/unit/test_agent_surface_contract.py` has a local `VALID_MODES` that omits
`"verify"` and will need aligning to the canonical set.
===============================================================================

Task: 2

Implement transparent reporting so responses no longer imply `mode` influenced
retrieval, and ensure the CLI surfaces this honesty signal.

- In `application/services/graph_service.py`, in `DefaultGraphService.resolve`
and `.search` (around lines 125 and 139), alongside the existing write of `mode`
into `metadata`, add an explicit honesty signal: `mode_applied: False` plus a
short `mode_note` explaining that the value was accepted but does not yet affect
retrieval depth in this version.
- Follow the existing in-code precedent of the "`task` accepted but ignored in
V1.5" comment style, annotating these lines so future readers understand the
field is intentionally inert pending real implementation.
- Update `_envelope_payload` (and the human renderer) in
`adapters/inbound/cli/commands/query.py` to surface `mode`, `mode_applied`, and
the note from `env.metadata`, so CLI JSON/human output no longer implies mode
was honored.
- Confirm the MCP tool (which already echoes `env.to_dict()["metadata"]`)
automatically carries these new fields through, and verify the not-implemented
flag is present in that payload.
===============================================================================

Task: 3

Update the surrounding surfaces (agent contract manifest, status
recommendations, and documentation) so none of them advertise or recommend
`mode` behavior that isn't implemented.

- In `domain/agent_context_port.py`, revise the manifest rule text that
currently promises escalation "to … deep" so it no longer implies mode changes
retrieval; clarify that mode is advisory/not-yet-honored. Keep the per-intent
recipe `mode` values in `CONTEXT_RESOLVE_RECIPES` unchanged, since they remain
intent-appropriate guidance for future implementation and are pinned by
curated-recipe contract tests.
- In `application/use_cases/report_status.py`, annotate the recommended recipe /
`recommended_next_actions` (specifically the `resolve` action) to carry the
honesty note that mode is not yet applied, so `potpie status` no longer
recommends a mode as if it were effective.
- Update `docs/context-graph/cli-flow.md` to clearly state that `--mode` is
validated against `fast|balanced|verify|deep` but does not yet change the read
path — strengthen the existing V1.5 caveat and align the version wording.
- Update the `context_resolve` tool docstring/parameter description in
`adapters/inbound/mcp/server.py`, and the `--mode` help text in `query.py`, to
reflect that the value is validated but not yet honored.
===============================================================================

Task: 4

Add test coverage for the new validation and honesty-signaling behavior, and
reconcile existing tests that had drifted from the canonical mode set.

- Add unit tests for the strict validator (raises `ValueError` on unknown
values, accepts each canonical value) near
`tests/unit/test_source_references.py`.
- Add tests asserting that `ResolveRequest`/`SearchRequest` construction rejects
an invalid `mode` with `ValueError`.
- Add CLI/MCP-level tests asserting an invalid `--mode`/`mode` is rejected: CLI
should follow the `validation_error` exit path; MCP should return `{"ok": False,
"error": "ValueError"}`. Also assert that a valid mode returns a response
containing the `mode_applied: False`/not-implemented signal.
- Update `VALID_MODES` in `tests/unit/test_agent_surface_contract.py` to match
the canonical `RESOLVE_MODES` set (including `"verify"`), and confirm
`test_agent_context_port.py` recipe assertions still hold since recipe `mode`
values are left unchanged.
- Adjust any `report_status`/manifest tests affected by the added honesty note.

💡 Iterate on the plan with: `@coderabbitai` <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!

[AdityaPainuli]

@nndn I'd love to work on this one.

My suggested approach, pending your call on which remedy you feel is right for this:

Option A (my preference as the first step): validate mode against the documented set at the boundary (reject unknown values with a clear error), and surface unimplemented modes transparently in the envelope instead of echoing them as honored - same honesty pattern the read path already uses for unsupported_include (reason not_implemented). Small, no retrieval semantics invented, kills the silent no-op on all three surfaces.

Option B (wire it): actually map modes to retrieval behavior (e.g., fast/ deep shaping max_items / verification depth). Happy to do this instead of or as a follow-up, but the semantics feel like a product decision you'd want to specify rather than have me guess.

Let me know your preference on this, I'll put up a PR.