fix: harden retrieval routing and runtime hints - #29143
Conversation
Add source-of-truth routing helpers and tests, then adopt builder retrieval-manners guidance to avoid raw retrieval block leakage, irrelevant context bleed, and unverified snippet claims.\n\nVerification:\n- python -m py_compile agent/prompt_builder.py tests/agent/test_prompt_builder.py\n- python -m pytest -o addopts= tests/agent/test_prompt_builder.py::TestGuidanceConstants::test_session_search_guidance_routes_between_memory_surfaces tests/agent/test_prompt_builder.py::TestRetrievalRouteHelper -q
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I verified that the underlying gap still exists on current main, but I would not salvage this older branch as-is.
Problems
- The route classifier is too broad:
agent/prompt_builder.py:304sends any prompt containingreview,task,decision,report, ortickettoshared_workwhenever that surface is available. That can misroute normal PR/code-review or task-execution prompts away from the live-system/session grounding this PR is trying to enforce. - The runtime
shared_workmapping inrun_agent.py:2179checks forfabric_recall,fabric_search, andfabric_pending, but current main has nofabric_*tool definitions (git grep -n "fabric_recall\|fabric_search\|fabric_pending" origin/main -- .returned no matches), so that branch is not grounded in a current tool surface. - This exact branch is mechanically stale:
gh pr view 29143 --json mergeStateStatusreportsDIRTY, and same-author #32206 is an open refreshed stack containing these commits plus later related work.
Suggested changes
- Narrow
shared_workmatching to explicit Fabric/shared-work phrasing, or remove it until the real tool names exist. - Add regression cases for ambiguous prompts like
review this PR,show this task status, andwhat did we decide last time.
Automated hermes-sweeper review.
| elif _contains_retrieval_marker(text, ("skill", "workflow", "procedure", "runbook", "how do we usually")): | ||
| preferred = "skills" | ||
| reason = "reusable workflows belong in skills/procedures" | ||
| elif _contains_retrieval_marker(text, ("fabric", "review", "task", "decision", "report", "ticket")): |
There was a problem hiding this comment.
This marker set is too broad for a routing primitive. Words like review, task, decision, report, and ticket appear in ordinary PR-review and task-execution prompts; when shared_work is available this would steer those prompts away from the live-system/session-search sources the PR is meant to enforce.
| surfaces.add("memory") | ||
| if "skill_manage" in names or "skill_view" in names: | ||
| surfaces.add("skills") | ||
| if {"fabric_recall", "fabric_search", "fabric_pending"} & names: |
There was a problem hiding this comment.
I could not find fabric_recall, fabric_search, or fabric_pending tool definitions on current main. Unless those names are introduced in the same stack, this shared_work surface is ungrounded and the branch is effectively dead in the current runtime.
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two PRs address the retrieval-routing gap. #29141 and its fresher rebase #29143 both add source-of-truth classification and API-only route hints, but the shared-work classifier is broader than the reported grounding need and references Fabric tool surfaces absent from current main.
Related pull requests
- #29141 [closed]
duplicate— (+596/-18) — duplicate: This earlier version adds retrieval guidance, route classification, API-only hint injection, and related tests, but it remains relevant only as the predecessor of #29143; it was closed after a contributor identified #29143 as the same fix rebased onto newer main at 6a6766f. - #29143
related— (+592/-12) — superseded: This rebase carries essentially the same retrieval-routing implementation as #29141, including the overbroad shared_work keywords and mappings for fabric_* tools that the contributor review found absent from current main. Despite the keep_open verdict recorded on the #29143 review, the review text says not to salvage this DIRTY branch as-is and identifies same-author #32206 as the refreshed stack containing these commits plus later related work.
Duplicates
#29141 and #29143 are substantively the same fix stack; #29141 was closed as a duplicate of #29143, and #29143 is in turn superseded by the refreshed same-author stack in #32206.
Suggested consolidation
Close #29143 as duplicate of #32206, making the chain explicit: #29141 → #29143 → #32206. This departs from the visible keep_open verdict on #29143 because the accompanying contributor review documents that the branch is DIRTY, should not be salvaged as-is, contains an overbroad shared_work classifier and ungrounded fabric_* mappings, and has already been refreshed in #32206; #29141 can remain closed as the older duplicate.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup29141 ["PRs duplicating each other"]
P29141["PR #29141 (closed)"]
P29143["PR #29143 (open)"]
end
class P29141 closed
class P29143 open
class P29143 target
click P29141 "https://github.com/NousResearch/hermes-agent/pull/29141"
click P29143 "https://github.com/NousResearch/hermes-agent/pull/29143"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 63 kB of PR diffs, 3 kB of issue/PR text, 2 kB of discussion (3 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
Summary
Supersedes #29141 with the same fix stack rebased onto the newer upstream
mainat6a6766fb8.This PR preserves two local Hermes fixes:
Fresh rebase note
Upstream moved again after #29141 was opened, causing a narrow conflict in
hermes_cli/kanban_db.pyaround additive index creation. The conflict was resolved by keeping the newer upstream migration block that creates all additive task indexes after their columns exist, includingidx_tasks_session_id.Verification
Targeted verification after the fresh rebase: