[codex] Add agentic RAG and local-first runtime - #23
Merged
Conversation
|
Ready to act? Review this PR in Change Stack to turn feedback into patch suggestions you can inspect and refine. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughImplements opt-in agentic RAG orchestration (LangGraph), durable reviewed eval-case persistence and export, grouped-citation validation/repair integrated into chat, ingest/worker commit control with Redis cache invalidation, frontend agentic UI, CI/deploy compose rendering, and a local-first runtime decision with comprehensive docs and tests. ChangesAgentic RAG, Eval Management, and Local-First Runtime
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
tomnguyen103
marked this pull request as ready for review
June 5, 2026 20:49
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
README.md (1)
264-277:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix VPS override filename mismatch in cloud deploy command.
Line 276 references
deploy/docker-compose.vps.yml, but the documented tracked file isdeploy/docker-compose.vps.yml.example(Line 205). This can break copy-paste deployment steps on a fresh clone unless you add an explicit “copy .example to .yml” step first.Suggested docs patch
-DC="docker compose -p second-brain -f deploy/docker-compose.prod.yml -f deploy/docker-compose.vps.yml --env-file deploy/.env.prod" +# First create your local override from the template: +# cp deploy/docker-compose.vps.yml.example deploy/docker-compose.vps.yml +DC="docker compose -p second-brain -f deploy/docker-compose.prod.yml -f deploy/docker-compose.vps.yml --env-file deploy/.env.prod"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 264 - 277, Update the cloud deployment docs where the DC variable is defined: either change the referenced override filename from deploy/docker-compose.vps.yml to deploy/docker-compose.vps.yml.example or add a prior instruction to copy deploy/docker-compose.vps.yml.example to deploy/docker-compose.vps.yml; ensure the README mentions the required step (e.g., "cp deploy/docker-compose.vps.yml.example deploy/docker-compose.vps.yml") so the DC command (docker compose -p second-brain -f deploy/docker-compose.prod.yml -f deploy/docker-compose.vps.yml --env-file deploy/.env.prod) works on a fresh clone.backend/app/jobs/worker.py (1)
61-72:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSet cache invalidation only after successful job completion.
Line 61 sets
invalidate_searchbeforequeue.mark_done(...). Ifmark_donethrows, the job can end as failed while Line 70 still bumpscache:search:epoch.Suggested fix
- result = handler(db, job.payload, embedder=embedder, llm=llm) - invalidate_search = isinstance(result, dict) and bool(result.get("searchable")) + result = handler(db, job.payload, embedder=embedder, llm=llm) if savepoint.is_active: savepoint.commit() queue.mark_done(db, job, result=result) + invalidate_search = isinstance(result, dict) and bool(result.get("searchable"))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/jobs/worker.py` around lines 61 - 72, The code computes invalidate_search before calling queue.mark_done, which can cause cache bumping even if mark_done later fails; change the logic to determine/infer invalidate_search only after the job has been successfully marked done (i.e., after queue.mark_done returns without throwing) and before db.commit, and only then call bump_search_cache_epoch(redis_client, cfg) when invalidate_search is true; update references around the savepoint/queue.mark_done/queue.mark_failed block (variables: invalidate_search, result, queue.mark_done, queue.mark_failed, bump_search_cache_epoch, savepoint, db.commit) so any exceptions from mark_done prevent cache invalidation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/app/api/chat.py`:
- Around line 123-132: The code calls deps.get_llm_client(...) before checking
req.options.agentic and settings.agentic_rag_enabled, causing disabled agentic
requests to surface provider/config errors; change the flow so the agentic gate
runs before initializing the LLM client: move or delay the call to
deps.get_llm_client until after the agentic branch check (i.e., check
req.options.agentic and if true validate settings.agentic_rag_enabled and raise
the 409 via HTTPException before calling deps.get_llm_client), and ensure
agentic_chat is only invoked after the client exists when needed.
In `@backend/app/demo/seed.py`:
- Line 69: Guard the access to capture.ingest.documents before indexing: check
that capture and capture.ingest.documents exist and are non-empty (e.g., if not
capture.ingest.documents: log/skip/return) before doing document =
capture.ingest.documents[0], then only proceed with indexing using that
document; this prevents the IndexError when ingest yields no documents and keeps
the seed command from crashing.
In `@docs/adr/0015-local-first-runtime.md`:
- Line 43: Replace the awkward phrase "needs paid 24/7 uptime" with a more
standard formulation such as "requires paid 24/7 uptime" or "requires continuous
24/7 paid uptime" in the sentence that currently reads "controls without
pretending a single-user assistant needs paid 24/7 uptime." Update that sentence
so it reads e.g. "controls without pretending a single-user assistant requires
24/7 paid uptime" to improve clarity and flow.
In `@docs/phase-6-plan.md`:
- Around line 3-6: Remove the blank line immediately after the opening
blockquote so the quoted paragraph starting with "**Runtime update
(2026-06-05):** ADR-0015 supersedes the VPS default..." is contiguous inside the
blockquote; this fixes markdownlint MD028 and prevents doc-lint failures by
ensuring the entire sentence remains a single quoted paragraph.
In `@docs/project-plan.md`:
- Line 32: Update the Redis cost wording in the table row that currently reads
"| **Cache / hot path** | **Redis** for embedding cache, query cache, rate
limiting | $0 (on the VM) | named: Redis |" to align with the local-first
default runtime framing used across the docs (e.g., indicate it is local-first /
included with the runtime rather than "on the VM" and keep the
$0/no-additional-cost semantics). Ensure you edit the same table row text so the
cost column reflects "local-first (included) — $0" or equivalent phrasing
consistent with other entries.
In `@docs/USAGE.md`:
- Around line 509-512: Update the API endpoint bullets to explicitly state the
path convention used (local direct routes vs VPS proxy) by annotating each
bullet like `POST /api/admin/retention/purge` and `POST
/api/feedback/eval-candidates/{feedback_id}/promote` with a short parenthetical
e.g. "(VPS proxy: /api/...)" or "(local: /...)" and similarly clarify the
example `http://localhost:8000/data/export` as a local direct route; ensure all
bullets in this section consistently follow the same notation so readers can
unambiguously tell when to use the /api prefix versus the localhost path.
In `@frontend/app/chat/page.tsx`:
- Around line 35-49: When route changes abort in-flight work via
abortRef.current?.abort(), guard any non-stream agentic completion resolvers
from calling finishAssistant(...) (and related state updates like setIsSending
and setMessages) if that abort has occurred: before invoking finishAssistant or
updating state in the completion success paths (the handlers around
finishAssistant at the locations referenced), check
abortRef.current?.signal?.aborted and skip/return early when true so stale
responses do not land in the new route context.
In `@frontend/components/ChatComposer.tsx`:
- Around line 75-87: The agentic toggle button (the motion.button using Brain
and state via agenticMode and setAgenticMode) lacks accessible labeling and
state for assistive tech; update that button to include an explicit aria-label
(e.g., "Toggle agentic RAG") and an aria-pressed attribute bound to agenticMode
(aria-pressed={agenticMode}) and ensure the title remains or is synchronized
with the aria-label so screen readers can identify its purpose and current
state.
---
Outside diff comments:
In `@backend/app/jobs/worker.py`:
- Around line 61-72: The code computes invalidate_search before calling
queue.mark_done, which can cause cache bumping even if mark_done later fails;
change the logic to determine/infer invalidate_search only after the job has
been successfully marked done (i.e., after queue.mark_done returns without
throwing) and before db.commit, and only then call
bump_search_cache_epoch(redis_client, cfg) when invalidate_search is true;
update references around the savepoint/queue.mark_done/queue.mark_failed block
(variables: invalidate_search, result, queue.mark_done, queue.mark_failed,
bump_search_cache_epoch, savepoint, db.commit) so any exceptions from mark_done
prevent cache invalidation.
In `@README.md`:
- Around line 264-277: Update the cloud deployment docs where the DC variable is
defined: either change the referenced override filename from
deploy/docker-compose.vps.yml to deploy/docker-compose.vps.yml.example or add a
prior instruction to copy deploy/docker-compose.vps.yml.example to
deploy/docker-compose.vps.yml; ensure the README mentions the required step
(e.g., "cp deploy/docker-compose.vps.yml.example deploy/docker-compose.vps.yml")
so the DC command (docker compose -p second-brain -f
deploy/docker-compose.prod.yml -f deploy/docker-compose.vps.yml --env-file
deploy/.env.prod) works on a fresh clone.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6df475b2-359a-4156-b8d4-887c708ce004
📒 Files selected for processing (76)
.github/workflows/ci.yml.gitignoreAGENTS.mdREADME.mdbackend/.env.examplebackend/README.mdbackend/app/agentic_rag/__init__.pybackend/app/agentic_rag/service.pybackend/app/api/chat.pybackend/app/api/conversations.pybackend/app/chat/prompt.pybackend/app/chat/service.pybackend/app/config.pybackend/app/dataops/retention.pybackend/app/db/models.pybackend/app/demo/__init__.pybackend/app/demo/seed.pybackend/app/eval/configs.pybackend/app/eval/dataset.pybackend/app/eval/export_cases.pybackend/app/eval/harness.pybackend/app/eval/metrics.pybackend/app/eval/pipeline.pybackend/app/ingest/service.pybackend/app/jobs/handlers.pybackend/app/jobs/worker.pybackend/app/main.pybackend/app/research/service.pybackend/app/schemas/chat.pybackend/migrations/versions/0005_eval_cases.pybackend/requirements.prod.txtbackend/requirements.txtbackend/tests/integration/test_api.pybackend/tests/integration/test_chat.pybackend/tests/integration/test_demo_seed.pybackend/tests/integration/test_jobs_worker.pybackend/tests/integration/test_rls.pybackend/tests/integration/test_search.pybackend/tests/unit/test_agentic_rag.pybackend/tests/unit/test_api_auth.pybackend/tests/unit/test_chat_citation_support.pybackend/tests/unit/test_config.pybackend/tests/unit/test_eval_configs.pybackend/tests/unit/test_eval_export_cases.pybackend/tests/unit/test_eval_metrics.pybackend/tests/unit/test_prompt.pydeploy/Dockerfile.frontenddeploy/docker-compose.prod.ymldeploy/docker-compose.vps.yml.exampledeploy/k8s/README.mddocs/PROGRESS.mddocs/USAGE.mddocs/adr/0001-llm-driver-local-vs-hosted.mddocs/adr/0005-hybrid-retrieval-rrf.mddocs/adr/0008-evaluation-and-mlflow.mddocs/adr/0011-vps-provider.mddocs/adr/0012-productionization-and-data-governance.mddocs/adr/0013-briefing-scheduling-and-worker.mddocs/adr/0014-kubernetes-learning-track.mddocs/adr/0015-local-first-runtime.mddocs/adr/0016-agentic-rag-v1.mddocs/adr/README.mddocs/case-study.mddocs/data-model/er-diagram.mddocs/implementation-notes.mddocs/k8s-evidence/11-teardown.txtdocs/phase-6-plan.mddocs/phase-7-plan.mddocs/project-plan.mdfrontend/.env.examplefrontend/app/chat/page.tsxfrontend/app/feedback/page.tsxfrontend/components/ChatComposer.tsxfrontend/components/ConversationSidebar.tsxfrontend/components/MessageList.tsxfrontend/lib/api/types.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the full local-first/agentic RAG update:
Root Cause / Why
Regular RAG was solid as a baseline, but exploratory prompts exposed three practical gaps: agentic orchestration needed to stay bounded and citation-safe, Gemini output sometimes used grouped citations or uncited framing text, and repeated local test chats made the sidebar noisy. The runtime docs also needed to reflect the actual preferred usage: local-first, no recurring infrastructure bill by default.
Validation
260 passed, 8 warningsnpm run lint,npm run build,npm audit --audit-level=highbaseline,agentic --no-mlflowat1.000hit/recall/citation/refusal on the fake-driver setkubectl kustomize deploy/k8s.github/workflows/*.ymluv pip check --python .\.venv\Scripts\python.exepython -m app.eval.export_cases --helppython -m app.demo.seed --helptr -d '\r' < deploy/cron/second-brain-backup | bash -ngit diff --cached --checkNotes
This PR intentionally keeps Agentic RAG false-by-default at both backend and frontend flags. It does not add recurring infrastructure cost.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation