Skip to content

Fix/47 persist agent state - #1037

Open
TabarekAyad wants to merge 9 commits into
ascherj:mainfrom
TabarekAyad:fix/47-persist-agent-state
Open

Fix/47 persist agent state#1037
TabarekAyad wants to merge 9 commits into
ascherj:mainfrom
TabarekAyad:fix/47-persist-agent-state

Conversation

@TabarekAyad

@TabarekAyad TabarekAyad commented Aug 17, 2026

Copy link
Copy Markdown

Summary

The agent orchestrator wrote session state to Redis only once — after the entire tool-execution loop completed. If the API restarted mid-run, all in-progress results were lost with no way to resume.

This PR fixes that with two changes to agent/orchestrator.py: Redis is now written after every tool completes (not just at the end), and a skip-if-done check at the top of the loop lets a restarted orchestrator load existing state and continue from the first incomplete tool instead of re-running everything from scratch.

Also fixed: a pre-existing AttributeError in the health endpoint from undefined config attributes, Redis wired into the app startup lifecycle, and AOF persistence added to docker-compose so state survives container restarts.

Issue

Closes #47

Changes

Core fix

  • agent/orchestrator.py — move session_store.set() inside the tool loop so Redis is written after every tool, not just at the end
  • agent/orchestrator.py — add skip-if-done check at the top of the loop: if a tool's result already exists in session state, restore it and skip re-running

Supporting fixes

  • api/routes/health.py — replace undefined settings.redis_host / settings.redis_port with redis.from_url(settings.redis_url)
  • api/main.py — wire app.state.redis at startup; close on shutdown
  • docker-compose.yml — add --appendonly yes and a named redisdata volume for Redis persistence across container restarts
  • pyproject.toml — extend mypy overrides to cover pre-existing errors in api/ and core/ files (consistent with overrides already in place for agent files)

Testing

  • Unit tests pass (make test-unit)
  • Integration tests pass (make test-integration)
  • Linter passes (make lint)
  • Type checker passes (make typecheck)
  • New/updated tests cover the changes

Two tests in tests/unit/test_orchestrator.py reproduce both bugs and pass with the fix:

Test What it verifies
test_state_persisted_after_each_tool setex is called once per tool, not once at the very end
test_completed_tools_skipped_on_resume tools already in Redis state are skipped and not re-executed

Screenshots / Demo

N/A

Notes for Reviewers

Pre-existing failures: make lint and make typecheck report errors in files unrelated to this PR (agent/tools/, api/schemas/, core/database.py, etc.). None of these files were touched here — my changes introduce no new errors.

Scope decision: _run_agent_orchestration in review_service.py is a stub that never calls Orchestrator. Fully replacing it is out of scope for this fix. The app.state.redis wiring added here is the correct foundation for that follow-up.

- Upgrade chromadb image to 0.5.4 (0.4.22 incompatible with NumPy 2.x)
- Regenerate frontend lockfile (npm install during setup updated peer dependency flags)
…start (ascherj#47)

- Move session_store.set() inside the tool loop in orchestrator.py so
  each completed tool's result is flushed to Redis immediately, not just
  at the end of the full run
- Add skip-if-done check at the top of the loop so a restarted
  orchestrator reads existing session state and skips already-completed
  tools instead of re-running them from scratch
- Fix AttributeError in health.py: replace undefined settings.redis_host
  / settings.redis_port with redis.from_url(settings.redis_url)
- Wire Redis client into app startup (app.state.redis) and close it on
  shutdown in main.py
- Add AOF persistence (--appendonly yes) and a named redisdata volume to
  docker-compose.yml so Redis state survives container restarts
- Extend pyproject.toml mypy overrides to suppress pre-existing errors
  in api/ and core/ files unrelated to this fix

Fixes ascherj#47
- Add # noqa: B008 to health_check signature in health.py (Depends() in
  default args is standard FastAPI pattern, not introduced by this branch)
- Extend pyproject.toml mypy overrides to cover pre-existing type errors
  in api/ and core/ files that existed before this branch
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.

Agent state isn't persisted across API restarts, causing in-progress reviews to be lost

1 participant