propose: hints-v2 — extend hints to resolve and to fuzzy-strategy neighbors signals#146
Conversation
32e9179 to
6308246
Compare
6308246 to
bc2d460
Compare
HumanBean17
left a comment
There was a problem hiding this comment.
Propose review — HINTS-V2
Verdict: Direction is sound and trace-motivated. ~85% landable as-is. Fix contract gaps (v1 inheritance, terminology, purity story) and tighten FUZZY_STRATEGY_SET policy before implementation. Fine to keep as draft until those are addressed; add plans/PLAN-HINTS-V2.md before PR-A lands.
What works
- Trace-driven scope — Two narrow gaps (
resolvehad no hints;neighborshad no signal on brownfield/fallback edges) extend v1 (#144) without reopening Shape 2 or per-row hints. - v1 frame respected — Meta-tier, cap discipline, no per-row
neighborshints, validationsuccess=Falsecarve-out (§7.15). - Strategy-over-confidence — Sensible;
attrs.strategyis already categorical onDECLARES_CLIENT,EXPOSES,HTTP_CALLS, etc. - UC table — UC16b/16c and awkward-case notes (UC8/UC10 collapse) show implementer-level thinking.
- 2-PR split — Resolve-first, neighbors-second; hints-only, no reindex.
- Appendix A — Wire-up matches current
mcp_hints.pystyle.
Must fix in propose before merge
1. UC6/UC7 and §6 PR-B say "routes" for DECLARES_CLIENT
DECLARES_CLIENT targets Client nodes, not Route. §6 PR-B ("layer_c_source route and annotation route") will send implementers to the wrong rel. Rename to clients in UC6/UC7, or switch the motivating case to EXPOSES if the trace was actually about routes.
2. v1 ≤120-char rule not inherited
v1 §7.6: rendered hints ≤120 chars; overlong templates drop. TPL_RESOLVE_NONE_TRY_SEARCH embeds the full identifier — long FQNs can exceed the cap. v2 has no decision, test, or truncation policy. Lock one: truncate identifier, cap in finalize_hint_list, or drop the hint when over 120.
3. Purity story contradicts v1 §7.18
v1 locked pagination hints on echoed output fields (FindOutput.limit), not call-site kwargs. v2 plumbs identifier / hint_kind via side payload (fine — same as find's hint_payload), but §2.6 ("pure function of the output object") and §7.10 oversell it. Either echo resolved_identifier (+ optional hint_kind) on ResolveOutput, or reword §2.6/§7.10 to "payload dict at call site per §3.1.1" and cite the find precedent.
4. §7.14 wording
Appendix treats hint_kind=None as symbol branch. §7.14 ("If either is missing or empty") wrongly implies both are required. Fix to: only identifier must be non-empty; hint_kind may be None.
5. §3.2 table vs Appendix A
Table mentions "identifier parses as method + path" / "service-qualified target" but Appendix only branches on hint_kind. Delete those triggers from the table or implement them.
6. Route/client templates are not concrete road signs
filter={path_prefix: …} violates v1 §2.1/§2.7 (one concrete call, no ellipsis). Derive a real filter fragment from the identifier, or lock a fixed template with an explicit substitution rule.
Policy / ontology
7. FUZZY_STRATEGY_SET placement — Repo convention puts closed vocabularies in java_ontology.py. The set mixes declaration strategies (layer_c_source, layer_b_fqn) and CALLS strategies (phantom, chained_receiver, …). Document that values come from graph builder strategy columns; consider VALID_FUZZY_EDGE_STRATEGIES (or equivalent) so hints don't invent strings.
8. layer_b_ann vs layer_b_fqn — Both are brownfield layers in build_ast_graph.py. Why is layer_b_fqn fuzzy but layer_b_ann "reliable"? Needs a one-sentence policy or the set will look arbitrary on the next layer change.
9. resolve none duplicates existing message — _resolve_build_output already sets message to "use search(query=...)". v2's search hint repeats that for symbol/hint_kind=None. Pick: hints only for route/client, generic message + specific hints, or "hints must not repeat message verbatim" in §7.
10. Wildcard identifiers — resolve_v2 returns status: none for */? via empty matches; v2 would still emit try search(query='…') with the wildcard. Add a UC or explicit no-hint rule if intentional.
Nits (non-blocking)
- Add
plans/PLAN-HINTS-V2.mdbefore implementation (repo culture). - README update when code lands.
- Extend
generate_hintsLiteralwith"resolve"in §6 PR-A. - UC8
chained_receiveron aDECLARES_CLIENT-only call is odd — consider a CALLS-focused UC for clarity.
Suggested checklist before "ready for implementation"
- Fix UC6/UC7/§6 PR-B: clients not routes (or change edge type).
- Add v1 §7.6 120-char decision + test.
- Resolve purity: echo on
ResolveOutputor honest payload contract. - Fix §7.14 (
identifieronly). - Align §3.2 with Appendix.
- Concrete route/client templates.
- One paragraph on
layer_b_annvslayer_b_fqn. - Wildcard + duplicate-message policy.
- Ontology/builder alignment note for
FUZZY_STRATEGY_SET.
Happy to re-review after a propose revision pass.
bc2d460 to
a0835b6
Compare
|
Thanks for the thorough review — all 10 substantive items and the 4 nits made the propose tighter. Applied in Substantive items
Nits
Branch |
HumanBean17
left a comment
There was a problem hiding this comment.
Second-pass review — after a0835b6
Verdict: Amendments land round-1 feedback well. Ready for implementation after a short consistency pass (copy drift, not design). Approve the propose once the items below are fixed.
Round-1 checklist
| Item | Status |
|---|---|
| UC6/UC7 clients not routes | Fixed in §4; TL;DR still wrong |
| 120-char rule | Fixed (§7.18, UC2b, Appendix A) |
resolved_identifier echo |
Good; §3.1 / §3.1.1 prose overclaims full output-only purity |
§7.14 hint_kind=None |
Fixed |
| §3.2 dead parse triggers | Fixed (seed-based) |
| Concrete route/client templates | Fixed (§3.1.2, {seed}) |
FUZZY_STRATEGY_SET in ontology |
Fixed (§7.19) |
layer_b_ann vs layer_b_fqn |
Fixed (§7.20) |
| Duplicate message / wildcards | Fixed (§7.21–22, UC2c) |
| UC8 on CALLS | Fixed |
The reply comment on this thread accurately describes what landed. No pushback needed on substance.
Remaining nits (~10 min in-doc)
-
TL;DR line 11 — still says
DECLARES_CLIENTreturned 3 routes; §4 UC6/UC7 correctly say clients. Align the TL;DR. -
§6 PR-B test summary (line 197) — still "layer_c_source route and annotation route" for a
DECLARES_CLIENTround-trip. Should be clients (onelayer_c_source, oneannotationonDECLARES_CLIENTedges). -
§3.1.1 (lines 58–63) — opens with "all sourced from output fields" then lists
hint_kindas call-site plumbed. Rephrase to the honestfind_v2hybrid: output fields +hint_kind/ seeds in payload per §3.1.2. -
§3.1 line 54 — "pure function of the output object alone" is still too strong while
hint_kindand seeds are payload-only. Soften to matchfind_v2, or echohint_kindonResolveOutput(optional polish). -
§7.6 vs §7.19 — §7.6 says CI classifies against
mcp_hints.py; §7.19 says ontology. Pick ontology (correct per your amendment) and align §7.6. -
§2.7 — still says fuzzy enum lives in
mcp_hints.py; update tojava_ontology.py. -
UC16c — row text still says
identifiermissing; should sayresolved_identifier. -
§6 PR-A test summary — add UC2b/2c/3b/4b/16b/16c (or "all resolve UC rows in §4"); current list is stale.
Non-blocking
plans/PLAN-HINTS-V2.mdbefore PR-A — still fine to defer.- Echoing
hint_kindonResolveOutputwould make purity prose literally true; not required for v2.
Once 1–8 are addressed, this propose is good to implement. Happy to do a final skim after one more push.
a0835b6 to
94d917f
Compare
|
All 8 nits applied in
Branch |
Builds on the landed v1 hints surface (#120 / #144). Two narrow extensions, motivated by a real-world agent trace where the agent struggled after
neighbors([method_id], 'out', ['DECLARES_CLIENT'])returned three brownfield-layer-C routes with no road signs, and afterresolve(...)returnedstatus: nonewith guidance only in prosemessage.Scope:
hints: list[str]toResolveOutput; emit onstatus: none(branched byhint_kind) andstatus: many(candidate count).status: oneemits nothing. Plumbing contract foridentifier/hint_kindis locked at §3.1.1 / Decision §7.14 — missing plumbing suppresses the hint rather than rendering a degraded template.attrs.strategyfalls in a lockedFUZZY_STRATEGY_SET(layer_c_source,layer_b_fqn,phantom,chained_receiver,overload_ambiguous,implicit_super), emit a meta-tier hint pointing the agent atattrs.strategyper row.Out of scope (locked decisions, §5):
neighbors(v1 §7.15 frame stays binding).resolve many(would need a new field onResolveOutput).19 use cases (§4, including UC16b for cap truncation and UC16c for missing plumbing). 16 locked decisions (§7). 2 PRs (§6). Wire-up snippets in Appendix A.
Marking as draft for review.