Skip to content

fix(session_search): add session diversity to CJK LIKE fallback - #38442

Open
sujianddd-dev wants to merge 1 commit into
NousResearch:mainfrom
sujianddd-dev:fix/cjk-session-search-diversity
Open

fix(session_search): add session diversity to CJK LIKE fallback#38442
sujianddd-dev wants to merge 1 commit into
NousResearch:mainfrom
sujianddd-dev:fix/cjk-session-search-diversity

Conversation

@sujianddd-dev

Copy link
Copy Markdown

What does this PR do?

Short CJK tokens (<3 chars, common in Chinese 2-char words like \u5199\u5165/\u94fe\u8def)
fall to the LIKE path, which orders by timestamp DESC \u2014 pushing all results
into the current/active session and hiding older matching sessions.

This wraps the LIKE query with ROW_NUMBER() OVER (PARTITION BY s.id) to keep
up to 5 messages per session, ensuring results span multiple sessions.

Related issues

Short CJK tokens (<3 chars, common in Chinese 2-char words like 写入/链路)
fall to the LIKE path, which orders by timestamp DESC — pushing all results
into the current/active session and hiding older matching sessions.

Wrap the LIKE query with ROW_NUMBER() OVER (PARTITION BY s.id) to keep
up to 5 messages per session, ensuring results span multiple sessions.

Fixes missing session matches for Chinese/CJK users.
See NousResearch#20494 NousResearch#11511 NousResearch#20499.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have tool/skills Skills system (list, view, manage) labels Jun 3, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for identifying a real recall-starvation path. Current main still orders the short-CJK LIKE fallback by newest timestamp before limiting (hermes_state.py:4982-5000), while discovery fetches a bounded 300-row scan and only then skips/deduplicates sessions (tools/session_search_tool.py:513-569). The per-session window cap directly addresses that ordering problem.

Problems

  • No regression test covers the reported path. The CJK tests in tests/test_hermes_state.py:1814-2020 validate matching, filters, snippets, and OR routing, but not a high-volume current session exhausting the discovery scan before an older matching session can surface.

Suggested changes

  • Add an end-to-end discovery test in tests/tools/test_session_search.py: seed more than _DISCOVER_SCAN_LIMIT newer short-CJK matches in the current session plus an older matching session, then assert discovery with current_session_id returns the older session.

This is an automated hermes-sweeper review.

Comment thread hermes_state.py
FROM messages m
JOIN sessions s ON s.id = m.session_id
WHERE {' AND '.join(like_where)}
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a regression test for this cap through session_search discovery: more than the 300-row scan limit of newer short-CJK matches in the current session plus an older matching session should still return the older session after the current lineage is filtered.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@teknium1 teknium1 added the area/sessions Session lifecycle, resume, persistence, history label Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants