Skip to content

fix(mirror): correct docstring - writes SQLite only, not JSONL - #48471

Closed
HeLLGURD wants to merge 1 commit into
NousResearch:mainfrom
HeLLGURD:fix/mirror-docstring-jsonl
Closed

HeLLGURD wants to merge 1 commit into
NousResearch:mainfrom
HeLLGURD:fix/mirror-docstring-jsonl

Conversation

@HeLLGURD

Copy link
Copy Markdown

Bug

mirror_to_session() in gateway/mirror.py documents:

Finds the gateway session that matches the given platform + chat_id,
then writes a mirror entry to both the JSONL transcript and SQLite DB.

But the implementation only calls _append_to_sqlite(session_id, mirror_msg) -
there is no JSONL write. The docstring promises a write path that does not
exist, which misleads anyone reading or debugging the mirror flow into looking
for a JSONL transcript that never gets written.

This is intentional behavior - sessions are stored in SQLite
(SessionDB / hermes_state.py), and the per-session transcript.jsonl is a
legacy format that the codebase only cleans up, never writes here. So the
code is correct and the docstring is wrong.

Fix

Update the docstring to describe what the function actually does:

Finds the gateway session that matches the given platform + chat_id,
then writes a mirror entry to the SQLite session database.

Docstring-only change; no behavior change. The SQLite-only write path is left
exactly as-is.

@alt-glitch alt-glitch added type/docs Documentation improvements P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery labels Jun 18, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the precise documentation correction. Current main still makes the inaccurate dual-store claim at gateway/mirror.py:38, while the actual persistence path calls only _append_to_sqlite() at gateway/mirror.py:79; that helper writes through SessionDB.append_message() at gateway/mirror.py:191-201. The proposed one-line change accurately describes that behavior, and existing coverage already asserts the SQLite writer is called (tests/gateway/test_mirror.py:166-179).

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 14, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Docstring already reads "Append a delivery-mirror message to the target session's SQLite transcript" on current main (gateway/mirror.py::mirror_to_session), so this landed independently. Thanks @HeLLGURD — your #48483 preview fix went in via #109134.

@teknium1 teknium1 closed this Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users type/docs Documentation improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants