v2.4.1 — prompt/LLM plumbing, citation renumbering, agent timeouts and faithfulness calibration - #13
Open
DNSdecoded wants to merge 20 commits into
Open
v2.4.1 — prompt/LLM plumbing, citation renumbering, agent timeouts and faithfulness calibration#13DNSdecoded wants to merge 20 commits into
DNSdecoded wants to merge 20 commits into
Conversation
An audit of the agent-node prompts surfaced five issues; none crashed, all
degraded output quality silently.
- query_planner: `resp.text` is unguarded — google-genai raises on
safety-blocked or parts-only responses. Route both call sites through the
existing rag.safe_extract_text, matching reflexion_evaluator.
- reflexion_evaluator: the Gemini JSON-retry accepted a system-instruction
argument and silently discarded it. Pass it through, as query_planner does.
- tool_selector: RETRY RULES were numbered 7-9 after ROUTING RULES ended at 4.
Renumbered 5-7 (prompt text only).
- report_runner / watch_runner: both prompts interpolated an ISO language code
("hi") where a language name belonged, and report_runner paired it with an
English worked example — so non-English requests came back in English. Both
now use lang_utils.get_language_name, and the example is labelled
format-only with the language requirement stated last.
- figure_captioner: the VLM call carried no system instruction. Added one.
Tests: query_planner survives a blocked response; plan_sections names the
language natively.
…e panel format_context numbers every retrieved paper, but the citation panel keeps only the papers the answer actually cites. An answer drawing on papers 1 and 4 of 4 therefore rendered "[1] ... [4]" beside a two-entry panel. Add rag.compact_citations: extract the citations, renumber the survivors to a dense 1..M in context order, and rewrite the answer's markers to match. Markers resolving to no paper are dropped rather than left dangling. The approach mirrors report_runner._remap_markers, which already did this for multi-section reports. Wired into every path that had the defect: - rag.answer_question and both strategy variants (translated answers are compacted before translation, so they inherit the dense numbering) - POST /agent/query — the session log, query log and response body all use the rewritten answer - the /agent SSE stream — compaction moved ahead of the chunk loop, since markers streamed before the sources event cannot be corrected afterwards
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe changes update agent evaluation, citation compaction, language prompts, provider streaming limits, graph timing, faithfulness scoring, and PDF retention. They also add configuration guidance and regression tests for response parsing, timeout behavior, citations, truncation, NLI limits, and ingestion. ChangesCitation alignment
Language-specific prompting
Model interaction controls
Streaming output limits
Faithfulness and reflexion controls
PDF retention and ingestion
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AgentRoute
participant compact_citations
participant Session
participant Client
AgentRoute->>compact_citations: generated answer and citation metadata
compact_citations-->>AgentRoute: compacted answer and sources
AgentRoute->>Session: append compacted answer
AgentRoute-->>Client: stream compacted answer and sources
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
watch_runner.py (1)
34-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression test for the digest prompt.
The supplied test covers
report_runner.plan_sections, but notwatch_runner._summarize; a future regression in this newly changed prompt would go unnoticed.🤖 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 `@watch_runner.py` around lines 34 - 35, Add a regression test covering watch_runner._summarize and its newly changed digest prompt, asserting the generated prompt includes the requested language name and concise-digest wording. Keep the existing report_runner.plan_sections test unchanged and follow its established test setup and assertion style.
🤖 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 `@rag.py`:
- Around line 884-887: Update the citation-compaction calls at rag.py:884-887,
rag.py:976-979, and rag.py:1125-1127 to pass only the first
context_data["chunks_used"] metadata entries, keeping validation aligned with
the chunks included by format_context(). Add a regression in
tests/test_rag.py:91-116 covering a citation marker that targets metadata
excluded by formatted-context truncation.
In `@report_runner.py`:
- Line 43: Update the title-language instruction near the language rendering
logic so the “not in English” clause is omitted when lang_name is English, while
retaining it for other languages. Ensure the default language no longer produces
a contradictory message.
- Around line 36-43: Update plan_sections() so its fallback for no usable
model-generated sections translates or selects the default section titles in the
requested language instead of returning English _DEFAULT_SECTIONS. Preserve the
existing fallback behavior and section limit while ensuring every returned title
follows the language identified by lang_utils.get_language_name(language).
---
Nitpick comments:
In `@watch_runner.py`:
- Around line 34-35: Add a regression test covering watch_runner._summarize and
its newly changed digest prompt, asserting the generated prompt includes the
requested language name and concise-digest wording. Keep the existing
report_runner.plan_sections test unchanged and follow its established test setup
and assertion style.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 00d5ce13-5edf-468e-a09c-dc5fa8df3d4f
📒 Files selected for processing (11)
agent/nodes/query_planner.pyagent/nodes/reflexion_evaluator.pyagent/nodes/tool_selector.pyfigure_captioner.pyrag.pyreport_runner.pyroutes/agent.pytests/test_agent.pytests/test_rag.pytests/test_report_routes.pywatch_runner.py
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile SummaryThis PR delivers three independent fix sets: prompt/LLM plumbing fixes (
Confidence Score: 5/5Safe to merge; all three fix sets are well-bounded, thoroughly tested, and degrade gracefully on failure. The citation-compaction path is guarded by try/except at every call site, so a regex failure falls back to the original answer. The three-gate time-budget logic in the reflexion evaluator is covered by dedicated tests that pin all three config knobs to prevent ambient .env interference. Provider timeout changes use separate client pools to avoid cross-contaminating streaming and non-streaming requests. The faithfulness recalibration is backed by measured positive/negative distributions and guarded by a test asserting the threshold stays within the reachable range. Files Needing Attention: No files require special attention. The reflexion_evaluator.py gate ordering and the compact_citations newline-drop logic are the subtlest changes and both have dedicated test coverage.
|
| Filename | Overview |
|---|---|
| rag.py | Adds compact_citations (renumbers answer markers to dense 1..M, drops dangling refs) and threads visible_chunks through extract_citations/compact_citations at every non-agent answer path. Logic is correct; newline-drop edge case in _repl is covered by new tests. |
| agent/nodes/reflexion_evaluator.py | Replaces single budget gate with three ordered checks (reserve, loop-budget, post-eval), threads system_instruction into the completeness LLM call, and adds NLI timing logs. Gate ordering is correct and well-tested. |
| config.py | Adds LLM_REQUEST_TIMEOUT_S, LLM_STREAM_TIMEOUT_S, AGENT_EVAL_RESERVE_S, NLI_MAX_SEQ_LENGTH, NLI_MAX_CHUNKS_PER_CITATION, AGENT_FAITHFULNESS_ACCEPT; raises LLM_MAX_TOKENS and AGENT_TIMEOUT defaults with measured justification. NLI_MAX_CHUNKS_PER_CITATION is correctly clamped to >=1 to prevent the fail-open faithfulness=1.0 footgun. |
| providers/gemini.py | Introduces a separate stream_pool (LLM_STREAM_TIMEOUT_S) vs. pool (LLM_REQUEST_TIMEOUT_S), both sharing the same _index cycle; detects MAX_TOKENS finish_reason and appends TRUNCATION_NOTE. Shared _index across both pools is intentional and correct since both have the same keys. |
| routes/agent.py | Moves compact_citations ahead of session/query logging in both agent_query and agent_stream so the stored answer, log entry, and response body all carry the corrected markers. Both call sites are inside try/except, so a failure gracefully falls back to the original answer. |
| sse_utils.py | Compacts citations before the done event (enabling client re-render), adds INTERRUPTED_NOTE for mid-stream deaths, and falls through to a done event instead of returning early when partial text exists. |
| verify.py | Sets model.max_seq_length at load time and caps cited chunks per paper via NLI_MAX_CHUNKS_PER_CITATION, reducing faithfulness pass from ~318s to ~25s. Both knobs are configurable and the cap is clamped >=1 to prevent silent disable. |
| report_runner.py | Fixes plan_sections to interpolate language name instead of ISO code; adds _default_sections with translation fallback; updates _remap_markers regex to capture leading whitespace so a dangling marker is dropped with its preceding space, matching rag._CITE_MARKER_RE behaviour. |
| watch_runner.py | Adds _keep_pdf to persist downloads as {paper_id}.pdf in PAPERS_DIR, fixing orphan chunks; fixes digest prompt to use language name. Fallback on OSError degrades to old behaviour. |
| agent/graph.py | Wraps every node in _timed() for wall-time logging; replaces six individual add_node calls with a loop. No logic changes to graph topology. |
Sequence Diagram
sequenceDiagram
participant C as Client
participant R as routes/agent.py
participant G as agent/graph.py
participant AP as query_planner
participant AG as answer_generator
participant RE as reflexion_evaluator
participant F as finalizer
C->>R: POST /agent/query
R->>G: build_agent_graph().invoke()
Note over G: _timed() wraps every node
G->>AP: query_planner_node
AP->>AP: safe_extract_text(resp)
AP-->>G: sub_queries, detected_language
loop tool_executor + answer_generator
G->>AG: answer_generator_node
AG-->>G: draft_answer, context_chunks_used
end
G->>RE: reflexion_evaluator_node
Note over RE: Gate 1 reserve check
Note over RE: Gate 2 loop budget count>=1
RE->>RE: NLI check_claims
RE->>RE: completeness LLM call
Note over RE: Post-eval budget check
alt accept or budget exhausted
RE-->>G: final_answer
G->>F: finalizer_node
F-->>G: final_answer
else retrieve_more or regenerate
RE-->>G: cycle back
end
G-->>R: state
R->>R: compact_citations
R-->>C: AgentQueryResponse
Reviews (12): Last reviewed commit: "diag(streaming): log why a stream died, ..." | Re-trigger Greptile
Both citation remappers returned '' for a marker whose numbers all resolved to no paper, leaving a doubled space mid-sentence and a trailing space at the end of a sentence. Capture the leading whitespace in the marker pattern so a fully-dangling marker is dropped together with the space in front of it. Also make plan_sections' language instruction conditional: with the default language="en" it rendered as "MUST be written in English, not in English".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@rag.py`:
- Around line 121-123: Update _CITE_MARKER_RE and its _repl() handling so fully
dangling citation markers remove any preceding line-break whitespace, including
the newline in “answer\n[99]”, while preserving intended spacing for
non-dangling markers. Add a regression test covering a marker at the start of a
line and verifying the result contains no trailing newline.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f1b0950f-a284-4309-8bc3-4fdd149bfe2e
📒 Files selected for processing (4)
rag.pyreport_runner.pytests/test_rag.pytests/test_report_routes.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_rag.py
max_output_tokens caps thinking and answer together. gemini-3.6-flash rejects thinking_budget=0, so providers/gemini.py drops thinking_config entirely and the model falls back to a dynamic budget that billed against the same 2048 ceiling. Measured on a real standard-RAG prompt (12 chunks, 3396 input tokens): thinking spent 0-4856 tokens across identical requests. At cap=2048 it took 1964 and left 80 for the answer, truncating mid-sentence; the answer itself needs <=2100 and the worst thinking+answer total was 6926. Raise the default to 8192, matching AGENT_MAX_TOKENS. Cost is unchanged for short queries, which still finish around 700 tokens at any cap.
Both backends yielded bare str and discarded finish_reason, so a stream cut short by the output token limit ended exactly like a completed one. llm_client, sse_utils and routes/chat.py all saw a normal finish and the answer simply stopped mid-sentence with no error and no warning. Track finish_reason in each generate_stream and append TRUNCATION_NOTE on MAX_TOKENS (Gemini) or "length" (OpenRouter). The text is the only channel that reaches the user without threading a new field through four layers, and generate_stream still yields str so no caller changes. Raising the cap alone would not have fixed this: thinking is dynamic, so the cliff is probabilistic rather than a fixed threshold.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@config.py`:
- Around line 260-264: Restore the separate token budgets: in config.py lines
260-264 set the LLM_MAX_TOKENS default to 2048 while retaining AGENT_MAX_TOKENS
at 4096; update .env.example line 48 to 2048, README.md line 422 to document
LLM_MAX_TOKENS as 2048 and AGENT_MAX_TOKENS as 4096, and docs/GEMINI_SETUP.md
line 133 to document LLM_MAX_TOKENS=2048.
In `@providers/base.py`:
- Around line 14-18: Update the shared TRUNCATION_NOTE constant in
providers/base.py so its guidance does not specifically reference
LLM_MAX_TOKENS; use neutral wording about raising the configured output-token
limit, or make the note dynamically reference the active token-limit setting
used by each caller.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eb86f9c4-8cfb-449f-bd4c-18fc73726a86
📒 Files selected for processing (9)
.env.exampleREADME.mdconfig.pydocs/GEMINI_SETUP.mdproviders/base.pyproviders/gemini.pyproviders/openrouter.pytests/test_providers_gemini.pytests/test_providers_openrouter.py
Comment on lines
+260
to
+264
| # Caps thinking + answer together, not just the answer. gemini-3.6-flash rejects | ||
| # thinking_budget=0 and spends 0-4856 thought tokens on identical prompts, so a | ||
| # 2048 cap left as little as 80 tokens for the answer and truncated mid-sentence. | ||
| # Measured: answer <=2100, worst thinking+answer 6926. | ||
| LLM_MAX_TOKENS = int(os.getenv("LLM_MAX_TOKENS", "8192")) # maximum tokens to generate |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Keep the LLM and agent token budgets at the required values.
The runtime default and all documented examples now use 8192 for LLM_MAX_TOKENS. Restore LLM_MAX_TOKENS=2048 and keep AGENT_MAX_TOKENS=4096 as a separate setting.
config.py#L260-L264: restore the runtime default to2048..env.example#L48-L48: set the example value to2048.README.md#L422-L422: document2048and correct the adjacent agent-budget documentation to4096.docs/GEMINI_SETUP.md#L133-L133: documentLLM_MAX_TOKENS=2048.
As per coding guidelines, keep AGENT_MAX_TOKENS at 4096 and LLM_MAX_TOKENS at 2048 as separate settings.
📍 Affects 4 files
config.py#L260-L264(this comment).env.example#L48-L48README.md#L422-L422docs/GEMINI_SETUP.md#L133-L133
🤖 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 `@config.py` around lines 260 - 264, Restore the separate token budgets: in
config.py lines 260-264 set the LLM_MAX_TOKENS default to 2048 while retaining
AGENT_MAX_TOKENS at 4096; update .env.example line 48 to 2048, README.md line
422 to document LLM_MAX_TOKENS as 2048 and AGENT_MAX_TOKENS as 4096, and
docs/GEMINI_SETUP.md line 133 to document LLM_MAX_TOKENS=2048.
Source: Coding guidelines
Owner
Author
There was a problem hiding this comment.
Not applying — this would revert a deliberate fix that is part of this same PR (e2dc9e2).
LLM_MAX_TOKENS caps thinking plus answer, not just the answer. gemini-3.6-flash rejects thinking_budget=0 and spends anywhere from 0 to 4856 tokens on thinking for identical prompts, so a 2048 cap left as little as ~80 tokens for the answer and truncated mid-sentence. Measured on this corpus: answer ≤2100 tokens, worst observed thinking+answer 6926. 8192 is sized from that measurement.
The 2048 figure appears in older documentation examples, which is presumably where the "required value" reading comes from — but it predates the switch to a thinking-capable model. The docs were updated to 8192 in this PR rather than the code being reverted to match them.
There was a problem hiding this comment.
@DNSdecoded, thank you for the detailed measurement. The model behavior explains why 8192 prevents truncated answers.
However, the active coding guidelines still explicitly require separate defaults: LLM_MAX_TOKENS=2048 and AGENT_MAX_TOKENS=4096. I cannot withdraw this finding while that requirement remains active.
Please update the coding guideline if 8192 is now the approved default. After that update, this change and its documentation are consistent.
(•ᴗ•)🐇
✏️ Learnings added
Learnt from: DNSdecoded
Repo: DNSdecoded/IndicRAG PR: 13
File: config.py:284-288
Timestamp: 2026-08-02T09:56:45.403Z
Learning: For this repository's Gemini configuration, `LLM_MAX_TOKENS` caps combined thinking and answer tokens. The maintainer reports that `gemini-3.6-flash` can consume 0–4,856 thinking tokens, and measured worst-case thinking-plus-answer output was 6,926 tokens. A 2,048-token limit can truncate answers; `LLM_MAX_TOKENS=8192` was deliberately selected for this model behavior.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
Comment on lines
+14
to
+18
| TRUNCATION_NOTE = ( | ||
| "\n\n*[Answer truncated — output token limit reached. " | ||
| "Ask a narrower question or raise `LLM_MAX_TOKENS`.]*" | ||
| ) | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 3 \
'AGENT_MAX_TOKENS|LLM_MAX_TOKENS|max_output_tokens|TRUNCATION_NOTE' \
--glob '*.py' --glob '*.md' --glob '.env.example'Repository: DNSdecoded/IndicRAG
Length of output: 157
🏁 Script executed:
#!/usr/bin/env bash
set -u
echo "Repository files matching likely names:"
git ls-files | rg '(^|/)(providers/base\.py|config\.py|.*agent.*\.py|.*stream.*\.py|.*\.py)$' | head -200
echo
echo "File existence check:"
for f in providers/base.py config.py; do
if [ -f "$f" ]; then echo "FOUND $f"; else echo "MISSING $f"; fi
done
echo
echo "Top-level Python-like files:"
git ls-files '*.py' | head -200Repository: DNSdecoded/IndicRAG
Length of output: 4094
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "providers/base.py:"
wc -l providers/base.py
cat -n providers/base.py
echo
echo "config.py relevant constants:"
rg -n -C 4 'AGENT_MAX_TOKENS|LLM_MAX_TOKENS|LLM_FALLBACK_MODEL' config.py
echo
echo "References to TRUNCATION_NOTE and gen_config:"
rg -n -C 3 'TRUNCATION_NOTE|gen_config|max_output_tokens|AGENT_MAX_TOKENS|LLM_MAX_TOKENS' \
--glob '*.py' --glob '*.md' --glob '.env.example' .Repository: DNSdecoded/IndicRAG
Length of output: 50377
Use the correct configuration name for agent truncation guidance.
Agent answer generation passes config.AGENT_MAX_TOKENS as max_output_tokens, so the shared TRUNCATION_NOTE should not always tell users to raise LLM_MAX_TOKENS. Use neutral wording such as “raise the configured output-token limit,” or pass the active config setting into the note.
🤖 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 `@providers/base.py` around lines 14 - 18, Update the shared TRUNCATION_NOTE
constant in providers/base.py so its guidance does not specifically reference
LLM_MAX_TOKENS; use neutral wording about raising the configured output-token
limit, or make the note dynamically reference the active token-limit setting
used by each caller.
Source: Coding guidelines
/papers and /ingest/health enumerate PAPERS_DIR and derive paper_id from
each file stem, so anything indexed without a matching PDF on disk was
invisible in the library panel and its chunks counted as orphans.
Two paths broke that invariant:
- watch_runner ingested from a temp download and deleted it, so a watched
paper never appeared at all and left orphan chunks behind. It now moves
the download into PAPERS_DIR as {paper_id}.pdf and ingests from there,
falling back to the temp path on OSError rather than losing the paper.
- routes/ingest saved URL/arXiv/DOI ingests under a title-derived filename
while indexing under _bibtex_safe_id(id). The stem never matched, so
those papers listed with 0 chunks and "Need re-index" despite indexing
fine, and DELETE /papers/{id} 404'd on them. Saving as {paper_id}.pdf
also drops the sanitize block and its fallback copy.
Filenames now read as ids rather than titles; the title still travels in
chunk metadata. Pre-existing orphan chunks are unaffected — their source
PDFs are gone, so they need a re-run or a purge.
Agentic mode returned "Agent pipeline timed out" after 10 minutes (AGENT_TIMEOUT=600), and every answer scored faithfulness ~0. Timeouts. No LLM call had an HTTP timeout: the Gemini client and the OpenRouter client both used SDK defaults (OpenAI: 600s x 2 retries), so a single stalled request outlived the whole agent budget, multiplied by the 3-attempt failover chain. Adds LLM_REQUEST_TIMEOUT_S (60s) to both clients. The reflexion wall-clock budget also only applied from iteration 2, so a slow first pass still paid a full NLI + completeness LLM cycle before anything noticed; it now applies whenever a draft exists. Latency. Per-node timing logging showed reflexion_evaluator at 317.7s of a 390s run, all of it in verify.check_claims. NLI cost is linear in pairs and premise length (measured: 1.15s/pair at 512 tokens, 0.4s at 256), and each sentence scored against every chunk of every paper it cited (~275 pairs). Truncates the premise to 256 tokens and caps chunks per cited paper at 2 (chunks are rerank-ordered, so the best support is first). Measured no quality cost: positive median 0.226 at 256 vs 0.221 at 512. Reflexion now runs ~88s. Faithfulness. FAITHFULNESS_THRESHOLD=0.5 was unreachable for this model. Calibrated on 20 real chunks: a sentence copied verbatim out of its own chunk scores median 0.226 / max 0.428, while an unrelated paper's sentence scores median 0.099 / p90 0.158. Recall at 0.5 was 0.00, so `grounded` was always False. 0.15 gives recall 0.70 at false-positive 0.10-0.15. The downstream gates were separately hardcoded at 0.75, which a fully grounded answer cannot reach either (per-claim recall 0.70 caps answers near 0.70), leaving accept-on-faithfulness, safe-stop and abstention as dead branches and deflating answer_confidence. They now share AGENT_FAITHFULNESS_ACCEPT (0.6). Observed after the fix: faith 0.03 -> 0.98, action=accept. The positive and negative distributions overlap, so ~1 in 7 unsupported claims still scores as grounded. Faithfulness is a usable signal, not a guarantee.
reflexion_evaluator is still the slowest node (~88s) and the wall-clock budget is 90s, so a slow run gets cut short at iteration 1. The node timing alone doesn't say whether the NLI pass or the completeness LLM call dominates now that the pair count is capped.
The previous change made AGENT_REFLEXION_BUDGET_S gate iteration 1 as well as later loops. On a CPU-only box the first pass alone runs past it (observed: 48s retrieval + 50s generation, reflexion entered at 191s), so every answer shipped unverified — no faithfulness, no completeness, no confidence, no abstention. That traded a timeout for a silent quality loss. The two gates protect different things and are now separate. The loop budget stops starting ANOTHER retrieve-generate-verify cycle and applies from iteration 2. A new AGENT_EVAL_RESERVE_S (90s) is the deadline guard: skip the evaluation only when too little remains under AGENT_TIMEOUT to finish it, since being killed mid-evaluation discards the draft and 504s.
Adds LLM_REQUEST_TIMEOUT_S, AGENT_EVAL_RESERVE_S, AGENT_FAITHFULNESS_ACCEPT, NLI_MAX_SEQ_LENGTH and NLI_MAX_CHUNKS_PER_CITATION, and corrects FAITHFULNESS_THRESHOLD to the calibrated 0.15. The threshold comment spells out the measurement, because it is the one value here that cannot be set by taste: it is a per-claim entailment probability from a specific model, and the previous 0.5 sat above every positive that model can produce. Also corrects AGENT_REFLEXION_BUDGET_S, which now applies from iteration 2 onwards rather than gating the first evaluation.
Adds LLM_REQUEST_TIMEOUT_S, AGENT_EVAL_RESERVE_S, AGENT_FAITHFULNESS_ACCEPT, NLI_MAX_SEQ_LENGTH and NLI_MAX_CHUNKS_PER_CITATION to the env table, and corrects FAITHFULNESS_THRESHOLD to the calibrated 0.15 with the measurement that justifies it. Also fixes three stale claims: the loop guardrail section described a single wall-clock budget (there are now three layers, and the loop budget deliberately does not gate the first evaluation), faithfulness was described as "minimum across claims" when it is the grounded fraction, and the retrieval section still named cross-encoder/nli-deberta-v3-base as the verification model after the switch to the multilingual default. Replaces the guessed agentic latency figures with measured ones and adds troubleshooting entries for pipeline timeouts and for faithfulness reading ~0 after an NLI model swap.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
config.py (1)
279-283: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winRestore the required token-budget split.
The runtime default and documentation use
8192where the required split isLLM_MAX_TOKENS=2048andAGENT_MAX_TOKENS=4096.
config.py#L279-L283: set the runtimeLLM_MAX_TOKENSdefault to2048..env.example#L48-L48: set the exampleLLM_MAX_TOKENSvalue to2048.README.md#L422-L423: documentLLM_MAX_TOKENS=2048andAGENT_MAX_TOKENS=4096.As per coding guidelines, keep
AGENT_MAX_TOKENS(4096) separate fromLLM_MAX_TOKENS(2048).🤖 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 `@config.py` around lines 279 - 283, Restore the required token-budget split by setting the LLM_MAX_TOKENS default to 2048 in config.py lines 279-283, updating .env.example lines 48-48 to 2048, and documenting LLM_MAX_TOKENS=2048 alongside AGENT_MAX_TOKENS=4096 in README.md lines 422-423; keep the two configuration values separate.Source: Coding guidelines
🧹 Nitpick comments (1)
tests/test_verify.py (1)
78-92: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winBuild the fixture from the configured cap.
range(8)only exercises truncation for some configuration values. If the cap becomes8, the test passes without observing a dropped chunk. If the cap becomes greater than8, the test fails even though the implementation can evaluate all eight chunks.Use
limit + 1chunks and assertlimitpairs.Proposed test adjustment
import config + limit = config.NLI_MAX_CHUNKS_PER_CITATION + assert limit > 0 + answer = "The framework uses deep Q-networks for optimization. [1]" - chunks = [f"chunk {i} about deep Q-networks" for i in range(8)] + chunks = [f"chunk {i} about deep Q-networks" for i in range(limit + 1)] metas = [{"title": "One Paper"} for _ in chunks] ... - assert len(called_pairs) == config.NLI_MAX_CHUNKS_PER_CITATION + assert len(called_pairs) == limit🤖 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 `@tests/test_verify.py` around lines 78 - 92, Update test_chunks_per_citation_are_capped to derive the fixture size from config.NLI_MAX_CHUNKS_PER_CITATION, creating limit + 1 chunks and matching metadata so truncation is always exercised. Assert that fake_model.predict receives exactly the configured limit of pairs.
🤖 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 `@agent/nodes/reflexion_evaluator.py`:
- Around line 107-112: Update the post-evaluation retry path in the reflexion
evaluator so an over-budget first evaluation (count == 0) finalizes the current
answer instead of returning regenerate or retrieve_more without a final_answer.
Apply the elapsed-time check after evaluation and before non-accept retry
actions, preserving normal retry behavior within budget; add a regression test
covering an over-budget first evaluation with a non-accept verdict.
In `@config.py`:
- Around line 293-297: Make LLM_REQUEST_TIMEOUT_S budget-aware so up to three
sequential generate_with_failover() attempts remain below
AGENT_REFLEXION_BUDGET_S and AGENT_TIMEOUT, using a safer default/example value;
update config.py and .env.example consistently, and revise README.md to document
the effective maximum fallback duration rather than only the per-attempt
timeout.
- Around line 298-308: Align AGENT_EVAL_RESERVE_S across config.py lines
298-308, .env.example lines 63-73, and README.md lines 566-569: either update
the documented runtime total to 120 seconds or reduce the reserve so the
documented 95-second retrieval/generation phase still leaves sufficient
evaluation time. Keep the default, examples, and documentation consistent so
reflexion_evaluator_node() does not skip the first verification pass.
In `@verify.py`:
- Around line 102-110: Update the cited_chunks construction used by check_claims
so each cited paper’s candidates are ranked by rerank score before applying
config.NLI_MAX_CHUNKS_PER_CITATION. Ensure rerank scores are available or
propagated into this flow, then retain only the highest-scoring chunks per
citation rather than slicing document order before best().
- Around line 106-110: Validate NLI_MAX_CHUNKS_PER_CITATION during configuration
loading as an integer whose value is at least 1, rejecting zero, negative
values, and invalid input before check_claims() can build cited_chunks. Preserve
the existing slicing behavior for valid positive values.
In `@watch_runner.py`:
- Around line 56-64: Coordinate retained-PDF publication with ingestion using a
shared per-paper retention-and-ingestion transaction: in watch_runner.py lines
56-64, replace the standalone move with that transaction, and in
routes/ingest.py lines 569-576, replace direct copying to the final retained
path with the same transaction. Stage each download at a unique temporary path,
hold shared per-paper synchronization through ingestion, and publish the final
{paper_id}.pdf only after ingestion is accepted.
---
Duplicate comments:
In `@config.py`:
- Around line 279-283: Restore the required token-budget split by setting the
LLM_MAX_TOKENS default to 2048 in config.py lines 279-283, updating .env.example
lines 48-48 to 2048, and documenting LLM_MAX_TOKENS=2048 alongside
AGENT_MAX_TOKENS=4096 in README.md lines 422-423; keep the two configuration
values separate.
---
Nitpick comments:
In `@tests/test_verify.py`:
- Around line 78-92: Update test_chunks_per_citation_are_capped to derive the
fixture size from config.NLI_MAX_CHUNKS_PER_CITATION, creating limit + 1 chunks
and matching metadata so truncation is always exercised. Assert that
fake_model.predict receives exactly the configured limit of pairs.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 471a59ed-0e84-4813-8c27-b66702f3a779
📒 Files selected for processing (14)
.env.exampleREADME.mdagent/graph.pyagent/nodes/finalizer.pyagent/nodes/reflexion_evaluator.pyconfig.pyproviders/gemini.pyproviders/openrouter.pyroutes/ingest.pytests/test_agent.pytests/test_verify.pytests/test_watch_run.pyverify.pywatch_runner.py
🚧 Files skipped from review as they are similar to previous changes (1)
- providers/openrouter.py
Comment on lines
+56
to
+64
| try: | ||
| config.PAPERS_DIR.mkdir(parents=True, exist_ok=True) | ||
| dest = config.PAPERS_DIR / f"{paper_id}.pdf" | ||
| shutil.move(tmp_path, dest) | ||
| logger.info("[Watch] saved %s", dest) | ||
| return str(dest) | ||
| except OSError as exc: | ||
| logger.warning("[Watch] could not save PDF for %s (%s) — indexing from temp", paper_id, exc) | ||
| return tmp_path |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Coordinate retained-PDF publication across ingestion flows.
Both flows can write {paper_id}.pdf while another flow ingests that same paper. This can make the retained PDF differ from the bytes used to create indexed chunks.
watch_runner.py#L56-L64: replace the standalone move with a shared per-paper retention-and-ingestion transaction.routes/ingest.py#L569-L576: use the same transaction instead of copying directly to the final retained path.
Stage downloads at unique paths. Hold shared per-paper synchronization through ingestion. Publish the final path only after the accepted ingestion result.
📍 Affects 2 files
watch_runner.py#L56-L64(this comment)routes/ingest.py#L569-L576
🤖 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 `@watch_runner.py` around lines 56 - 64, Coordinate retained-PDF publication
with ingestion using a shared per-paper retention-and-ingestion transaction: in
watch_runner.py lines 56-64, replace the standalone move with that transaction,
and in routes/ingest.py lines 569-576, replace direct copying to the final
retained path with the same transaction. Stage each download at a unique
temporary path, hold shared per-paper synchronization through ingestion, and
publish the final {paper_id}.pdf only after ingestion is accepted.
CI caught this: with no .env present the defaults are AGENT_TIMEOUT=120 and AGENT_EVAL_RESERVE_S=90, leaving 30s of headroom. The measured first pass on CPU is ~95s (45s retrieval + 50s generation), so the reserve gate fired on every stock-config run and the answer shipped with no faithfulness score — exactly the failure the reserve was added to avoid, just moved from timeout to silent skip. Raises the default to 300s, which fits the measured pipeline plus the ~25s evaluation. The test that caught it passed locally only because this machine's .env sets AGENT_TIMEOUT=600. It now pins all three knobs so ambient configuration cannot decide the outcome either way.
DNSdecoded
changed the title
Both found in PR review. NLI_MAX_CHUNKS_PER_CITATION fails open. A 0 (or negative) value slices away every cited chunk, check_claims() returns no claims, and the evaluator reads an empty claim list as "no citable claims != hallucination" — faithfulness 1.0, answer accepted with zero grounding. A typo in .env would silently disable verification while reporting perfect scores. Clamped to >=1 at config load. An over-budget first evaluation could still start another cycle. The gate at the top of the node deliberately lets iteration 1 evaluate even when already past AGENT_REFLEXION_BUDGET_S, but a retrieve_more or regenerate verdict there returned no final_answer, so the graph ran a full retrieve->generate cycle (~95s on CPU) that the budget exists to prevent — only the next entry to the node would have stopped it. The retry path now finalises the evaluated draft when the budget is spent.
format_context truncates the context by chunk count and by total length, but every citation call site held the FULL retrieved metadata. A number the model invented past the truncation point therefore resolved to a real retrieved paper it was never shown, and the answer carried a citation that looked legitimate — worse than a dangling marker, which at least gets dropped. extract_citations/compact_citations take an optional visible_chunks, and number only that slice so such a marker dangles and is dropped. Wired through every path that resolves citations, not just the one reported: - rag.answer_question and both strategy variants (context_data ['chunks_used']) - /query/stream and /chat/stream via sse_stream (prepared['chunks_used']) - /agent/query and the agent SSE stream — answer_generator now surfaces chunks_used as AgentState['context_chunks_used'], since the count lived only inside that node Reported by CodeRabbit on rag.py; the agent and SSE paths had the same defect and were not flagged.
…back outline Both from PR review. A dangling marker alone on its own line left its newline behind, so dropping it produced a blank line that markdown renders as a paragraph break. The leading newline is now captured and removed with the marker, but only when the marker ends the line — otherwise it is restored, or removing a mid-line marker would splice the neighbouring lines together. Applied to report_runner._remap_markers as well, which mirrors this regex and had the same behaviour. plan_sections promised section titles in the requested language but returned the English _DEFAULT_SECTIONS on the fallback path, breaking that promise exactly when the planner had already failed. The fallback outline is now translated with one short call; English stays the last resort if that fails too, and the English path makes no extra call.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.env.example (1)
318-343: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEvaluate the NLI score range before accepting the threshold.
The values parse correctly,
verify.pyand the agent finalizer use the sameAGENT_FAITHFULNESS_ACCEPT, andNLI_MAX_CHUNKS_PER_CITATIONis clamped to at least 1. KeepAGENT_FAITHFULNESS_ACCEPT=0.6only if a fully grounded answer can reach it withFAITHFULNESS_THRESHOLD=0.15.🤖 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 @.env.example around lines 318 - 343, Validate the faithfulness calibration represented by FAITHFULNESS_THRESHOLD and AGENT_FAITHFULNESS_ACCEPT: confirm that a fully grounded answer can reach the 0.6 acceptance threshold at a 0.15 NLI cutoff. Retain AGENT_FAITHFULNESS_ACCEPT=0.6 only if that outcome is supported; otherwise adjust it to a reachable value while preserving the existing parsing and enforcement settings.config.py (1)
284-288:⚠️ Potential issue | 🟠 MajorRestore the separate token budgets.
LLM_MAX_TOKENSdefaults to8192, andAGENT_MAX_TOKENSis also8192at Line 289. The required defaults areLLM_MAX_TOKENS=2048andAGENT_MAX_TOKENS=4096. These defaults feed both non-streaming and streaming LLM calls, so the current values remove the intended budget separation.As per coding guidelines, keep
AGENT_MAX_TOKENSat4096andLLM_MAX_TOKENSat2048as separate settings.Proposed fix
-LLM_MAX_TOKENS = int(os.getenv("LLM_MAX_TOKENS", "8192")) +LLM_MAX_TOKENS = int(os.getenv("LLM_MAX_TOKENS", "2048")) -AGENT_MAX_TOKENS = int(os.getenv("AGENT_MAX_TOKENS", "8192")) +AGENT_MAX_TOKENS = int(os.getenv("AGENT_MAX_TOKENS", "4096"))🤖 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 `@config.py` around lines 284 - 288, Restore the separate token-budget defaults in the configuration constants: set LLM_MAX_TOKENS to default to 2048 and keep AGENT_MAX_TOKENS at 4096. Preserve both settings as distinct environment-backed values so their existing non-streaming and streaming call sites use the intended budgets.Source: Coding guidelines
🤖 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 `@sse_utils.py`:
- Around line 73-76: Buffer the answer instead of emitting raw chunks in the SSE
flow around the existing answer emission and extract_citations call. Invoke
rag.compact_citations with assembled, metadatas, and visible_chunks, then emit
its compacted answer and returned citations; for Strategy B, translate the
compacted answer before emission.
---
Outside diff comments:
In @.env.example:
- Around line 318-343: Validate the faithfulness calibration represented by
FAITHFULNESS_THRESHOLD and AGENT_FAITHFULNESS_ACCEPT: confirm that a fully
grounded answer can reach the 0.6 acceptance threshold at a 0.15 NLI cutoff.
Retain AGENT_FAITHFULNESS_ACCEPT=0.6 only if that outcome is supported;
otherwise adjust it to a reachable value while preserving the existing parsing
and enforcement settings.
In `@config.py`:
- Around line 284-288: Restore the separate token-budget defaults in the
configuration constants: set LLM_MAX_TOKENS to default to 2048 and keep
AGENT_MAX_TOKENS at 4096. Preserve both settings as distinct environment-backed
values so their existing non-streaming and streaming call sites use the intended
budgets.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ade46049-dca4-449c-a147-5279f247d832
📒 Files selected for processing (14)
.env.exampleREADME.mdagent/nodes/answer_generator.pyagent/nodes/reflexion_evaluator.pyagent/state.pyconfig.pyrag.pyreport_runner.pyroutes/agent.pyroutes/chat.pyroutes/query.pysse_utils.pytests/test_agent.pytests/test_rag.py
🚧 Files skipped from review as they are similar to previous changes (3)
- routes/agent.py
- tests/test_agent.py
- README.md
43ab52b moved citation compaction ahead of the chunk loop for /agent/stream, but the shared sse_utils path used by /query/stream and /chat/stream never got it — it only built the citation list at the end. A streamed answer citing papers 1 and 4 of 4 therefore rendered "[1] ... [4]" beside a two-entry panel, and after the visible_chunks change a marker past the prompt's truncation point stayed on screen with no source behind it. Chunks still stream live, so the raw markers still go out; the done event now carries the compacted answer and the client re-renders from it. The compaction happens before translation, so a translated answer inherits the dense numbering (same order rag.answer_question uses). /chat/stream also persisted the raw streamed text to session history, so follow-up turns inherited the gapped markers — it now saves the compacted answer. Reported by CodeRabbit on sse_utils.py.
…rity Chunks stream before citation numbering can be resolved, so a client that concatenates them gets text whose [N] markers disagree with the citations it is handed. Documents the event table and states plainly that `done`'s `answer` is the one to render. Also records the citation-integrity guarantee the last few commits added: markers resolve only against the chunks that actually reached the prompt, so a number invented past the truncation point can no longer resolve to a real paper the model was never shown.
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
…stream Regression from dee1226. That commit gave the Gemini client pool a 60s HTTP timeout to stop a stalled call from consuming the agent budget, but generate_stream draws from the same pool — and for google-genai the timeout covers the WHOLE stream, not the gap between chunks. A long standard-mode answer therefore had its socket torn down partway through: "[WinError 10054] An existing connection was forcibly closed by the remote host", leaving a truncated answer on screen. Streaming now has its own budget (LLM_STREAM_TIMEOUT_S, 300s): a second Gemini client pool, and a per-request override on the OpenRouter call so the client default still governs unary requests. The same report showed citations missing. That is a second defect: the SSE error branch returned before the done event, so a stream that died partway left the user with the partial answer and no sources at all. The done event is now still emitted whenever there is text to attribute, and only a completely empty answer stops early.
The comment said to keep LLM_REQUEST_TIMEOUT_S "well under AGENT_REFLEXION_BUDGET_S so failover still fits inside the budget". It does not: generate_with_failover walks up to 3 (provider, model) attempts sequentially, so 60s per attempt is ~180s worst case against a 90s budget. The value is right and the comment was wrong. Agent answer generation is a unary call measured at 20-50s on CPU, so lowering the timeout to ~30s to make the arithmetic true would abort real generations instead of stalled ones. What actually bounds the chain is AGENT_TIMEOUT plus AGENT_EVAL_RESERVE_S, which finalise the draft rather than 504, and the per-(provider, model) circuit breaker makes three consecutive full stalls rare. Records the deadline-aware alternative for whoever needs the worst case bounded properly. Same correction applied to .env.example and the two README spots that repeated it, plus a floor on the troubleshooting advice to lower it.
WinError 10054 still cuts long standard-mode answers after 9a82bf3, so the stream-timeout theory was at best incomplete. Rather than guess again, log what tells the causes apart at the point of failure: elapsed seconds, characters emitted, the configured limit, and the exception type. Elapsed near LLM_STREAM_TIMEOUT_S means our own timeout cut it; elapsed well under it means the provider dropped the connection. Separately, the salvaged answer now says it is incomplete. The done event carries the partial text plus citations, so once the error toast is dismissed a response that stops mid-sentence reads as a finished one. INTERRUPTED_NOTE is distinct from TRUNCATION_NOTE, which means the token limit was reached rather than the connection lost.
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
Two independent fix sets, both surfaced by auditing the agent-node prompts and one live agentic query.
1. Prompt and LLM-plumbing defects (
d1b6af1)Five issues, none of which crashed — all degraded output quality silently.
agent/nodes/query_planner.pyresp.textunguarded; google-genai raises on safety-blocked or parts-only responsesrag.safe_extract_textagent/nodes/reflexion_evaluator.pyquery_plannerdoesagent/nodes/tool_selector.pyreport_runner.py,watch_runner.py"hi") where a language name belonged;report_runnerpaired it with an English worked example, so non-English requests came back in Englishlang_utils.get_language_name; example relabelled format-only, language requirement stated lastfigure_captioner.py2. Citation numbering (
43ab52b)format_contextnumbers every retrieved paper, but the citation panel keeps only the papers the answer actually cites. An answer drawing on papers 1 and 4 of 4 rendered[1] ... [4]beside a two-entry panel.New
rag.compact_citationsextracts the citations, renumbers survivors to a dense1..Min context order, and rewrites the answer markers to match; markers resolving to no paper are dropped rather than left dangling. This mirrorsreport_runner._remap_markers, which already did the same for multi-section reports.Wired into every affected path:
rag.answer_questionand both strategy variants — translated answers are compacted before translation, so they inherit the numberingPOST /agent/query— session log, query log and response body all use the rewritten answer/agentSSE stream — compaction moved ahead of the chunk loop, since markers streamed before the sources event cannot be corrected afterwards3. Agent pipeline timeouts and faithfulness scoring (
dee1226…df5e9e7)Reported symptom: agentic mode returned "⚠ Agent pipeline timed out." Diagnosis found three separate defects behind it, plus one this work introduced and CI caught.
Nothing bounded a single LLM call.
providers/gemini.pyandproviders/openrouter.pyboth built clients with no HTTP timeout, so the SDK defaults applied (OpenAI: 600s × 2 retries) and one stalled request outlived the whole agent budget — multiplied by the 3-attempt failover chain. NewLLM_REQUEST_TIMEOUT_S(60s) bounds every call so failover fires instead of hanging.No per-node timing existed, so "which node ate the budget" could only be inferred. Each node is now wrapped once in
build_agent_graphand logs[Graph] <node> took Ns. That measurement drove everything below.The NLI faithfulness pass cost 317.7s of a 390s run. Cost is linear in pairs and premise length (measured: 1.15s/pair at 512 tokens, 0.4s at 256), and each sentence scored against every chunk of every paper it cited — ~275 pairs. Truncating the premise (
NLI_MAX_SEQ_LENGTH=256) and capping chunks per cited paper (NLI_MAX_CHUNKS_PER_CITATION=2) brings it to ~25s, with no measured quality cost (positive median 0.226 at 256 vs 0.221 at 512).Faithfulness read ~0 on every answer ever produced.
FAITHFULNESS_THRESHOLD=0.5was unreachable for the configured model. Calibrated on 20 real chunks — positive = a sentence copied verbatim out of its own chunk, negative = a sentence from a different paper:Recall at 0.5 was 0.00. At 0.15 it is 0.70 at false-positive 0.10–0.15. The downstream gates were separately hardcoded at
0.75, which a fully grounded answer cannot reach either (per-claim recall 0.70 caps answers near 0.70), leaving accept-on-faithfulness, safe-stop and abstention as dead branches and deflatinganswer_confidence. They now shareAGENT_FAITHFULNESS_ACCEPT(0.6).Verified live:
faith=0.03 → 0.96,action=accept, reflexion317.7s → 25.3s. An adversarial check separates cleanly — real claims score 0.327/0.372, fabricated ones 0.113/0.149.Two follow-on defects, both caught rather than shipped:
AGENT_REFLEXION_BUDGET_Sgate iteration 1 as well as later loops. On CPU the first pass alone exceeds it, so every answer shipped unverified — a timeout traded for a silent quality loss. The budget now stops only further cycles; a separateAGENT_EVAL_RESERVE_Sguards the deadline.AGENT_TIMEOUT=120, reserve 90) leave 30s of headroom against a ~95s first pass, so verification was skipped on every default-config run.AGENT_TIMEOUTdefault raised to 300s. The test had passed locally only because this machine's.envsets 600; it now pins all three knobs so ambient config cannot decide the outcome.Known limitation, stated deliberately: the positive and negative distributions overlap, so roughly 1 in 7 unsupported claims still scores as grounded. Faithfulness is a usable signal, not a guarantee. Tightening it needs a better NLI model or sentence-window scoring, not a threshold tweak.
Test plan
python -m pytest tests -q→ 276 passedpython figure_captioner.py→ self-check passestest_compact_citations_closes_numbering_gaps— 4 papers, cites 1 and 4, incl. a[1, 4]multi-markertest_compact_citations_drops_dangling_markertest_query_planner_survives_safety_blocked_responsetest_plan_sections_prompt_names_language_nativelytest_first_evaluation_runs_even_past_the_loop_budget— the loop budget must not skip iteration 1test_evaluation_skipped_when_timeout_reserve_is_gone— no NLI/LLM call when the deadline cannot fit onetest_faithfulness_threshold_is_reachable— guards against a threshold above what the NLI model can producetest_chunks_per_citation_are_capped— 8 chunks of one paper cost 2 NLI pairs, not 8Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation