galaxyproject / galaxyproject/brc-analytics

Auto-save: drop redundant Redis re-read, User load, history copies and refreshes per turn

Open
#1,714 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
7
Forks
11
Avg merge
3d 15h
Merged PRs (30d)
15

Description

Part of #1707. Found in the review of #1689.

## Problem

The signed-in auto-save path in `backend/api/app/api/v1/assistant.py` (around line 176) does redundant work on every turn:

- **Third session read.** It re-reads the session from Redis via `get_session` and runs `SessionState.model_validate` over the whole history, milliseconds after `_run_turn` wrote it (`assistant_agent.py:1194`).
- **Full `User` load.** `analysis_store.py:114` loads a whole `User` row where `get_user_id_by_keycloak_sub` exists and `turn_log.py:96` already uses it.
- **Double deep copy of the history.** `analysis_store.py:123–124` runs `to_jsonable_python` and then `strip_nuls` over `agent_message_history`, which is already `List[Dict]` per `models/assistant.py:233`. That's two full traversals of the largest per-session blob.
- **Pointless refresh.** `crud.py:261` / `crud.py:274` `session.refresh()` the full row after commit, re-selecting three JSON columns, even though `expire_on_commit=False` and the only consumer reads `.id`. `crud.py:360–362` already explains why this is unnecessary.

Related cleanup: `_get_saved_analysis_by_id` (`crud.py:183`) duplicates `get_saved_analysis`, and its post-rollback justification applies to no reachable path.

## Suggested fix

Hand the state out of `_run_turn` via an `on_state` callback like `on_turn`, pass `user_id` instead of loading `User`, drop the `to_jsonable_python` on the history, drop both refreshes, and fold `_get_saved_analysis_by_id` into `get_saved_analysis`.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the signed-in auto-save path in backend/api/app/api/v1/assistant.py and _run_turn in assistant_agent.py, then inspect analysis_store.py, turn_log.py, and crud.py at the referenced lines. Trace how state, user_id, history, refreshes, and saved-analysis lookup flow through these paths. Done means the redundant Redis read, User load, history traversal, refreshes, and duplicate lookup are removed as described.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, redis
Domain
api, backend, database
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.