|
| 1 | +# HINTS-STRING-REMOVAL-PROPOSE |
| 2 | + |
| 3 | +## Status |
| 4 | +Proposal — not yet implemented. |
| 5 | + |
| 6 | +## Problem Statement |
| 7 | +Two parallel hint fields (`hints: list[str]` and `hints_structured: list[StructuredHint]`) coexist on all five MCP tool outputs, carrying largely the same information in different formats. |
| 8 | + |
| 9 | +This redundancy creates concrete problems: |
| 10 | + |
| 11 | +1. **Maintenance burden** — every hint trigger requires dual emission (string template + structured object) in `mcp_hints.py`, with a parity test ensuring they stay in sync. |
| 12 | +2. **Information asymmetry** — non-actionable hints lose advisory context in structured form. A string like `"results look weak — narrow the query"` becomes `StructuredHint(tool="find", args={}, actionable=False)` with no explanatory text, while the string version carries the full advisory. |
| 13 | +3. **Wider surface than needed** — the project principles explicitly warn against widening public surface "just in case" and state that breaking changes are always allowed. |
| 14 | + |
| 15 | +The original string `hints` field predates `hints_structured` (added in #209). Now that structured hints are established and include `label` (#217), the string field's only unique contribution is the advisory reason text — which should simply be a field on `StructuredHint`. |
| 16 | + |
| 17 | +## Proposed Solution |
| 18 | +Consolidate to a single hint mechanism: |
| 19 | + |
| 20 | +1. **Add `reason: str` to `StructuredHint`** — carries the advisory text previously only in string hints (e.g. `"no match — try ranked fuzzy lookup"`, `"results look weak — narrow the query"`). |
| 21 | +2. **Remove `hints: list[str]`** from all five output models in `mcp_v2.py`. |
| 22 | +3. **Remove string hint generation** — `generate_hints()` returns only `list[_StructuredHint]`; all string template constants and `MCP_HINTS_FIELD_DESCRIPTION` are deleted. |
| 23 | +4. **Remove parity test** — `test_structured_hints_parity_with_string_hints` no longer applies. |
| 24 | + |
| 25 | +### `StructuredHint` after change |
| 26 | + |
| 27 | +```python |
| 28 | +class StructuredHint(BaseModel): |
| 29 | + label: str = "" |
| 30 | + tool: Literal["search", "find", "describe", "neighbors", "resolve"] |
| 31 | + args: dict[str, Any] |
| 32 | + actionable: bool = True |
| 33 | + reason: str = "" |
| 34 | +``` |
| 35 | + |
| 36 | +### `generate_hints` after change |
| 37 | + |
| 38 | +```python |
| 39 | +def generate_hints( |
| 40 | + output_kind: Literal["search", "find", "describe", "neighbors", "resolve"], |
| 41 | + payload: dict[str, Any], |
| 42 | +) -> list[_StructuredHint]: # was tuple[list[str], list[_StructuredHint]] |
| 43 | +``` |
| 44 | + |
| 45 | +### Example: actionable hint (describe finds no match) |
| 46 | + |
| 47 | +Before (two fields): |
| 48 | +```python |
| 49 | +{ |
| 50 | + "results": [], |
| 51 | + "hints": [ |
| 52 | + "no match — try search(query='BankAccount') for ranked fuzzy lookup", |
| 53 | + "try find(role='service') to browse by role" |
| 54 | + ], |
| 55 | + "hints_structured": [ |
| 56 | + {"tool": "search", "args": {"query": "BankAccount"}, "actionable": True, "label": ""}, |
| 57 | + {"tool": "find", "args": {"role": "service"}, "actionable": True, "label": ""} |
| 58 | + ] |
| 59 | +} |
| 60 | +``` |
| 61 | + |
| 62 | +After (single field): |
| 63 | +```python |
| 64 | +{ |
| 65 | + "results": [], |
| 66 | + "hints_structured": [ |
| 67 | + { |
| 68 | + "tool": "search", |
| 69 | + "args": {"query": "BankAccount"}, |
| 70 | + "actionable": True, |
| 71 | + "label": "", |
| 72 | + "reason": "no match — try ranked fuzzy lookup" |
| 73 | + }, |
| 74 | + { |
| 75 | + "tool": "find", |
| 76 | + "args": {"role": "service"}, |
| 77 | + "actionable": True, |
| 78 | + "label": "", |
| 79 | + "reason": "browse by role to discover related symbols" |
| 80 | + } |
| 81 | + ] |
| 82 | +} |
| 83 | +``` |
| 84 | + |
| 85 | +### Example: non-actionable advisory hint |
| 86 | + |
| 87 | +Before: |
| 88 | +```python |
| 89 | +{ |
| 90 | + "hints": ["results look weak — narrow the query or try find(role=…)"], |
| 91 | + "hints_structured": [{"tool": "find", "args": {}, "actionable": False}] |
| 92 | +} |
| 93 | +``` |
| 94 | + |
| 95 | +After: |
| 96 | +```python |
| 97 | +{ |
| 98 | + "hints_structured": [ |
| 99 | + { |
| 100 | + "tool": "find", |
| 101 | + "args": {"role": "service"}, |
| 102 | + "actionable": False, |
| 103 | + "label": "", |
| 104 | + "reason": "results look weak — narrow the query or try find with a role filter" |
| 105 | + } |
| 106 | + ] |
| 107 | +} |
| 108 | +``` |
| 109 | + |
| 110 | +## Scope |
| 111 | +- `mcp_hints.py` — remove string return, string templates, `MCP_HINTS_FIELD_DESCRIPTION`; add `reason` to `_StructuredHint`; update all hint templates to emit `reason` |
| 112 | +- `mcp_v2.py` — remove `hints: list[str]` from all five output models; add `reason: str = ""` to `StructuredHint`; update `_to_structured_hints` conversion |
| 113 | +- `tests/test_mcp_hints.py` — remove parity test; update assertions to check `reason` content instead of string hints |
| 114 | +- `README.md`, `docs/AGENT-GUIDE.md`, `AGENTS.md` — remove references to `hints` string field |
| 115 | + |
| 116 | +## Schema / Ontology / Re-index impact |
| 117 | +- Ontology bump: not required |
| 118 | +- Re-index required: no — this is an output-only change with no graph or index schema impact |
| 119 | +- Config/tool surface changes: `hints` field removed from all five tool outputs; `hints_structured` gains `reason` field |
| 120 | + |
| 121 | +## Tests / Validation |
| 122 | +- Existing hint tests rewritten to assert `reason` content on structured hints instead of string hints |
| 123 | +- Parity test removed (no longer applicable) |
| 124 | +- String-hint-specific test cases migrated to structured-hint assertions |
| 125 | +- Full test suite must pass — confirms no regressions in hint generation logic |
| 126 | +- Agent-facing behavior unchanged: structured hints already carry all tool-call information; `reason` adds advisory context |
| 127 | + |
| 128 | +## Open Questions ([TBD]) |
| 129 | +1. Should `reason` default to `""` or be required (`str` without default)? — Recommended: default `""` for backward compatibility during transition and to avoid forcing a reason on every hint. |
| 130 | +2. Should non-actionable hints always carry a `reason`? — Recommended: yes, non-actionable hints without a reason are noise and should be reconsidered at authoring time. |
| 131 | +3. Should string hint templates be deleted entirely or archived for reference? — Recommended: delete entirely; git history preserves them. |
| 132 | + |
| 133 | +## Out of scope |
| 134 | +- Changing hint trigger logic or priority tiers |
| 135 | +- Adding new hint categories or triggers |
| 136 | +- Modifying the `_StructuredHint` `priority` field semantics |
| 137 | +- Changing the `label` field behavior (added in #217) |
| 138 | + |
| 139 | +## Sequencing / Follow-ups |
| 140 | +Single implementation PR covering all files listed in scope. |
0 commit comments