Skip to content

fix: align AI-recent notes with WebUI prefill hook - #3037

Merged
7 commits merged into
nesquena:masterfrom
AJV20:webui-context-prefill-status
May 28, 2026
Merged

7 commits merged into
nesquena:masterfrom
AJV20:webui-context-prefill-status

Conversation

@AJV20

@AJV20 AJV20 commented May 28, 2026

Copy link
Copy Markdown
Contributor

Thinking Path

The WebUI-specific prefill hook is now the explicit browser-chat context path, but the third-party notes drawer's AI-recent Joplin helper still inspected only the legacy generic prefill_messages_script. That can make the drawer point at stale or no recall notes even when browser chat is correctly using webui_prefill_messages_script.

What Changed

  • Prefer webui_prefill_messages_script when deriving Joplin notes that are recently used by AI recall.
  • Fall back to the legacy prefill_messages_script for deployments that have not opted into the WebUI hook.
  • Support argv-style hooks such as ["python3", "/path/to/recall.py"] by selecting the script argument instead of the interpreter.
  • Added a regression test covering WebUI-hook precedence over the legacy hook.
  • Added a changelog entry.

Why It Matters

This keeps the notes drawer aligned with the same recall path browser-originated WebUI turns use, which makes WebUI context/notes behavior closer to the Telegram/CLI runtime expectation without hard-coding any note provider or local path.

Contract Routing

Task type: context / notes metadata parity.
Touched areas:

  • api/routes.py Joplin AI-recent note discovery
  • tests/test_webui_notes_sources.py
  • CHANGELOG.md
    Relevant public docs:
  • AGENTS.md
  • CONTRIBUTING.md
  • docs/CONTRACTS.md
    Scope boundaries:
  • Does not execute recall scripts.
  • Does not expose prefill message bodies.
  • Does not change chat streaming or the WebUI prefill loader.
    Evidence needed before claiming done:
  • Focused notes-source regression passes.
  • Existing WebUI prefill-context regression remains green.

Verification

  • python3.11 -m py_compile api/routes.py
  • python3.11 -m pytest tests/test_webui_notes_sources.py tests/test_webui_prefill_context.py -q -o addopts= (24 passed)
  • git diff --check

Risks / Follow-ups

  • Low risk: the change only selects which configured script file is inspected for existing Joplin note ID constants.
  • If a hook is a complex shell command without a script-like argument, the helper falls back to the last argv element rather than executing anything.

@AJV20

AJV20 commented May 28, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed this branch against current origin/master after the latest release batch moved the base and resolved the CHANGELOG.md conflict.

Verification on head 571bb101:

  • python3.11 -m py_compile api/routes.py
  • python3.11 -m pytest tests/test_webui_notes_sources.py
  • git diff --check

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Summary

Read the diff at PR head 571bb10 (28 added lines in api/routes.py, 33 in tests) and cross-checked the recall hook contract by reading api/streaming.py:300-360. The fix is correct: the Joplin AI-recent drawer was inspecting the wrong config key for browser-originated WebUI turns. CI green on 3.11/3.12/3.13.

Code reference

The browser chat recall path on api/streaming.py:346-347:

def _load_prefill_messages_script(config_data: dict) -> dict:
    script_raw = os.getenv("HERMES_WEBUI_PREFILL_MESSAGES_SCRIPT", "") or config_data.get("webui_prefill_messages_script")

The Joplin "AI-recent" helper on master at api/routes.py:12888-12894 was inspecting a different key:

def _joplin_prefill_script_path() -> Path | None:
    cfg = get_config()
    path_value = cfg.get("prefill_messages_script") if isinstance(cfg, dict) else None

So the notes drawer was reading prefill_messages_script while browser chat was reading webui_prefill_messages_script. Whenever a deployment configured the WebUI-specific hook (which is what _load_prefill_messages_script actually executes for browser turns), the drawer fell through to a stale or missing legacy script and surfaced incorrect "AI-recent" notes.

The new precedence at api/routes.py:12909-12918 (post-fix):

return _script_path_from_config_value(
    cfg.get("webui_prefill_messages_script") or cfg.get("prefill_messages_script")
)

mirrors the contract exactly.

Argv handling

The new _script_path_from_config_value() correctly handles list-form hooks like ["python3", "/path/to/recall.py"] — important because webui_prefill_messages_script is documented to accept both string and argv-list forms, and _prefill_script_command() in api/streaming.py:317-328 supports both. The helper preferring .py / .sh / .bash extensions over the interpreter is the right heuristic; bare commands fall back to the last argv token, which is conservative.

One small note: the helper does not execute anything — it only does a Path(...).expanduser() for later read_text() regex scanning for stable Joplin note IDs (*_ID = "<hex>"). Even on a malicious-looking path the worst outcome is the file not existing or the regex not matching; no subprocess.run of the worktree-supplied path. That keeps this change low-risk.

Test coverage

tests/test_webui_notes_sources.py::test_joplin_recent_ai_notes_prefers_webui_prefill_script_hook installs BOTH keys in config (legacy + WebUI argv-list form), stubs _joplin_api_get to fail loudly if the legacy note ID is requested, and asserts the resolved titles match only the WebUI script's IDs. Tight and precise.

The existing legacy-only test (test_joplin_recent_ai_notes_uses_configured_prefill_script) still passes per the PR description's 24 passed run, which confirms the fallback path didn't regress.

Diagnosis / Recommendation

LGTM. The change is small, the contract alignment is correct, and the test exercises the precedence directly. Nothing in the diff requires running the worktree to verify, just reading. The merge resolves the CHANGELOG.md conflict cleanly per AJV20's refresh comment.

One nit-worthy follow-up (not blocking): there are two trailing blank lines added before def test_external_notes_ui_uses_minimal_lucide_icons_for_ai_recent_notes() in the tests file — PEP 8 wants exactly two, the diff shows three. Cosmetic, not worth a churn.

@AJV20

AJV20 commented May 28, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up after reviewing this PR against the WebUI prefill loader contract:

  • Mirrored the HERMES_WEBUI_PREFILL_MESSAGES_SCRIPT environment override in the AI-recent Joplin helper, matching browser chat's actual prefill precedence.
  • Parsed string-form hooks with shlex.split, so commands like python3 /path/to/recall.py select the recall script instead of the interpreter.
  • Added regression coverage for the env override/string command case and updated the changelog wording.

Verification on head 1b5e6f6f:

  • python3.11 -m py_compile api/routes.py
  • python3.11 -m pytest tests/test_webui_notes_sources.py tests/test_webui_prefill_context.py -q -o addopts= (25 passed)
  • git diff --check origin/master...HEAD

@AJV20

AJV20 commented May 28, 2026

Copy link
Copy Markdown
Contributor Author

Added a follow-up after a provider-neutrality pass:

  • Kept the WebUI prefill hook alignment provider-neutral in changelog wording.
  • Expanded notes-source regression coverage so configured third-party sources such as Joplin, Obsidian, Notion, and llm-wiki remain visible before runtime tool inventory hydrates.

Verification: python3.11 -m py_compile api/routes.py; python3.11 -m pytest tests/test_webui_notes_sources.py tests/test_webui_prefill_context.py -q -o addopts=; git diff --check origin/master...HEAD.

@AJV20

AJV20 commented May 28, 2026

Copy link
Copy Markdown
Contributor Author

Code-review follow-up: fixed the path parsing edge case found during review. Plain configured script paths that already exist are now treated as literal paths before falling back to shlex.split(...), so unquoted paths containing spaces keep working while command strings like python3 /path/to/recall.py still resolve correctly.

Verification: python3.11 -m py_compile api/routes.py; python3.11 -m pytest tests/test_webui_notes_sources.py tests/test_webui_prefill_context.py -q -o addopts=; git diff --check origin/master...HEAD.

@AJV20

AJV20 commented May 28, 2026

Copy link
Copy Markdown
Contributor Author

CI follow-up: the Python 3.13 matrix exposed an existing git fixture assumption that temporary repos start on master. I made the workspace git test fixture initialize temporary repos with git init -b master when supported, with a fallback for older git.

Verification: python3.11 and python3.13 focused notes/workspace-git tests passed locally; git diff --check origin/master...HEAD passed.

@nesquena-hermes nesquena-hermes closed this pull request by merging all changes into nesquena:master in 6267716 May 28, 2026
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Shipped in v0.51.154 / Release DZ (stage-batch36, commit 6267716). Thanks for the contribution!

ai-ag2026 pushed a commit to ai-ag2026/hermes-webui that referenced this pull request May 28, 2026
# Conflicts:
#	CHANGELOG.md
ai-ag2026 pushed a commit to ai-ag2026/hermes-webui that referenced this pull request May 28, 2026
9-PR medium-risk cleanup:
- nesquena#3037 routes.py: argv-style prefill hook + env-var override for notes drawer
- nesquena#3046 models.py: compression parent not repaired as stale interrupted turn
- nesquena#3048 session_discoverability.py: --repair-safe CLI with default dry-run
- nesquena#3053 ui.js: streaming KaTeX guard for parser-owned equations
- nesquena#3059 models.py: empty partial activity rows excluded from sidebar recency
- nesquena#3060 profiles.py: API key writes to .env (chmod 600), not config.yaml
- nesquena#3064 routes.py: MEDIA: image tokens allow exact session-referenced paths
- nesquena#3069 models.py: cron sessions with project_id surface via Cron Jobs chip
- nesquena#3077 gateway_chat.py: HTTP 401 maps to gateway_auth_error event
SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
# Conflicts:
#	CHANGELOG.md
SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
9-PR medium-risk cleanup:
- nesquena#3037 routes.py: argv-style prefill hook + env-var override for notes drawer
- nesquena#3046 models.py: compression parent not repaired as stale interrupted turn
- nesquena#3048 session_discoverability.py: --repair-safe CLI with default dry-run
- nesquena#3053 ui.js: streaming KaTeX guard for parser-owned equations
- nesquena#3059 models.py: empty partial activity rows excluded from sidebar recency
- nesquena#3060 profiles.py: API key writes to .env (chmod 600), not config.yaml
- nesquena#3064 routes.py: MEDIA: image tokens allow exact session-referenced paths
- nesquena#3069 models.py: cron sessions with project_id surface via Cron Jobs chip
- nesquena#3077 gateway_chat.py: HTTP 401 maps to gateway_auth_error event
bernyforce pushed a commit to bernyforce/hermes-webui that referenced this pull request Jul 29, 2026
# Conflicts:
#	CHANGELOG.md
bernyforce pushed a commit to bernyforce/hermes-webui that referenced this pull request Jul 29, 2026
9-PR medium-risk cleanup:
- nesquena#3037 routes.py: argv-style prefill hook + env-var override for notes drawer
- nesquena#3046 models.py: compression parent not repaired as stale interrupted turn
- nesquena#3048 session_discoverability.py: --repair-safe CLI with default dry-run
- nesquena#3053 ui.js: streaming KaTeX guard for parser-owned equations
- nesquena#3059 models.py: empty partial activity rows excluded from sidebar recency
- nesquena#3060 profiles.py: API key writes to .env (chmod 600), not config.yaml
- nesquena#3064 routes.py: MEDIA: image tokens allow exact session-referenced paths
- nesquena#3069 models.py: cron sessions with project_id surface via Cron Jobs chip
- nesquena#3077 gateway_chat.py: HTTP 401 maps to gateway_auth_error event
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants