feat(observability): add reranker scores, streaming model fix, eval mode tracing#55
Conversation
…val mode plumbing DEBT-014: rerank() returns RerankResult with all_scores for every evaluated candidate (not just top_k). Trace metadata now includes per-chunk reranker and original scores for post-hoc threshold analysis. BUG-019: capture resolved model name from litellm streaming chunks via CompletionChunk.model field. Streaming and non-streaming traces now report the same llm_model value. DEBT-015: when VEKTRA_EVAL_MODE=true, query_rewrite StepTrace includes original_query and rewritten_query text for diagnosing rewrite quality. DEBT-009: when VEKTRA_DEBUG_LOG_QUERIES=true, _rewrite_query() emits a debug-level structlog event with both original and rewritten text. FEAT-019: when VEKTRA_EVAL_MODE=true, build_prompt StepTrace includes the full messages list sent to the LLM for prompt inspection. Both eval_mode and debug_log_queries are added to QueryPipelineConfig (read from env vars), avoiding constructor changes to either pipeline. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add 4 tests for DEBT-015/FEAT-019 eval_mode gating: - rewritten query captured in trace when eval_mode=True - rewritten query excluded when eval_mode=False - full prompt messages captured when eval_mode=True - messages excluded when eval_mode=False Fix H1: rename `os` loop variable to `orig` in rerank scores comprehension to avoid shadowing the builtin. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fvadicamo
added
enhancement
|
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:
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 ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds pipeline flags Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Pipeline as Pipeline
participant Reranker as Reranker
participant LLM as LLMProvider
participant Tracer as Tracer
Client->>Pipeline: execute(question)
activate Pipeline
Pipeline->>Tracer: emit query_rewrite step (rewritten?/original/rewrite)
Pipeline->>Reranker: rerank(results, top_k)
activate Reranker
Reranker->>Reranker: evaluate all candidates -> all_scores
Reranker-->>Pipeline: RerankResult(top_k, all_scores)
deactivate Reranker
Pipeline->>Tracer: emit rerank step (candidates_evaluated, scores)
Pipeline->>Tracer: emit build_prompt step (messages when eval_mode)
Pipeline->>LLM: stream(messages, model)
activate LLM
LLM->>LLM: resolve model from first chunk
LLM-->>Pipeline: CompletionChunk(content, model)
deactivate LLM
Pipeline->>Tracer: emit llm_stream step (resolved model)
Pipeline-->>Client: QueryTrace(steps, llm_model)
deactivate Pipeline
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an evaluation mode and debug logging to the query pipelines, enabling the capture of original/rewritten queries and full prompt messages within traces. It also refactors the reranker service to provide full score visibility for all candidates and enhances LLM model resolution during streaming. Feedback highlights a security violation regarding PII in debug logs, a recommendation to gate detailed reranker metadata behind the evaluation flag to avoid production trace bloat, and a logic improvement in the LiteLLM provider to correctly capture the model name from the stream.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
vektra-core/src/vektra_core/advanced_pipeline.py (1)
657-684:⚠️ Potential issue | 🟡 MinorInclude
modelon failedllm_streamsteps too.The success path records the resolved model, but the exception path drops it from
llm_streammetadata. Adding it there keeps failure traces just as actionable.💡 Suggested change
steps.append( StepTrace( name="llm_stream", duration_ms=_elapsed_ms(t0), - metadata={"error": str(exc)}, + metadata={"model": llm_model, "error": str(exc)}, ) )As per coding guidelines,
vektra-core/**: RAG engine component. Check for QueryTrace completeness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vektra-core/src/vektra_core/advanced_pipeline.py` around lines 657 - 684, The exception branch for the llm_stream currently omits the resolved model in StepTrace metadata; in the except block capture or default llm_model (use the existing llm_model variable or self._llm_config.provider) and include it in the StepTrace metadata alongside the error (e.g., metadata should contain both "error": str(exc) and "model": llm_model) so failed llm_stream traces mirror the success path; update the StepTrace creation in the except block accordingly (symbols: llm_model, StepTrace, _llm_config.provider, stream exception block).vektra-core/src/vektra_core/reranker.py (1)
58-63:⚠️ Potential issue | 🟠 MajorFix reranker mock return type in
test_reranking_narrows_results().Line 287 mocks
reranker.rerankto return a bare list, but the caller reads.top_kand.all_scoresattributes (lines 297 and 304 in advanced_pipeline.py). Change the return value toRerankResult(top_k=[results[2]], all_scores=[])to match the new signature.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vektra-core/src/vektra_core/reranker.py` around lines 58 - 63, Update the test mock in test_reranking_narrows_results so that reranker.rerank returns an instance of RerankResult instead of a bare list: mock reranker.rerank to return RerankResult(top_k=[results[2]], all_scores=[]) so the caller in advanced_pipeline.py (which accesses .top_k and .all_scores) receives the expected object; ensure the test imports or references the RerankResult type used by reranker.rerank.
🧹 Nitpick comments (1)
vektra-core/src/vektra_core/pipeline.py (1)
503-517: Consider extracting prompt-trace metadata assembly into a shared helper.
build_metaandstream_build_metaare effectively duplicated; a helper would reduce drift between execute and streaming paths.♻️ Suggested refactor
+def _build_prompt_trace_metadata( + prompt_version: str, + selected_chunks: int, + selected_history: int, + messages: list[Message], + eval_mode: bool, +) -> dict[str, object]: + meta: dict[str, object] = { + "prompt_version": prompt_version, + "chunks_in_prompt": selected_chunks, + "history_turns_in_prompt": selected_history, + } + if eval_mode: + meta["messages"] = [{"role": m.role, "content": m.content} for m in messages] + return metaAlso applies to: 808-822
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vektra-core/src/vektra_core/pipeline.py` around lines 503 - 517, The prompt-trace metadata assembly is duplicated between build_meta and stream_build_meta; extract this logic into a shared helper (e.g., a private method like _assemble_prompt_meta) that takes the common inputs (self._renderer.prompt_version, selected_chunks, selected_history, messages, self._eval_mode and elapsed ms from t0) and returns the metadata dict; then replace the inline build_meta and stream_build_meta constructions with calls to that helper and reuse the resulting dict in the StepTrace creation in both execute and streaming paths (references: build_meta, stream_build_meta, StepTrace).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@vektra-core/src/vektra_core/advanced_pipeline.py`:
- Around line 657-684: The exception branch for the llm_stream currently omits
the resolved model in StepTrace metadata; in the except block capture or default
llm_model (use the existing llm_model variable or self._llm_config.provider) and
include it in the StepTrace metadata alongside the error (e.g., metadata should
contain both "error": str(exc) and "model": llm_model) so failed llm_stream
traces mirror the success path; update the StepTrace creation in the except
block accordingly (symbols: llm_model, StepTrace, _llm_config.provider, stream
exception block).
In `@vektra-core/src/vektra_core/reranker.py`:
- Around line 58-63: Update the test mock in test_reranking_narrows_results so
that reranker.rerank returns an instance of RerankResult instead of a bare list:
mock reranker.rerank to return RerankResult(top_k=[results[2]], all_scores=[])
so the caller in advanced_pipeline.py (which accesses .top_k and .all_scores)
receives the expected object; ensure the test imports or references the
RerankResult type used by reranker.rerank.
---
Nitpick comments:
In `@vektra-core/src/vektra_core/pipeline.py`:
- Around line 503-517: The prompt-trace metadata assembly is duplicated between
build_meta and stream_build_meta; extract this logic into a shared helper (e.g.,
a private method like _assemble_prompt_meta) that takes the common inputs
(self._renderer.prompt_version, selected_chunks, selected_history, messages,
self._eval_mode and elapsed ms from t0) and returns the metadata dict; then
replace the inline build_meta and stream_build_meta constructions with calls to
that helper and reuse the resulting dict in the StepTrace creation in both
execute and streaming paths (references: build_meta, stream_build_meta,
StepTrace).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 039573d0-678e-4eb7-adfb-1a3ca070e808
📒 Files selected for processing (10)
vektra-core/src/vektra_core/advanced_pipeline.pyvektra-core/src/vektra_core/pipeline.pyvektra-core/src/vektra_core/providers/litellm_provider.pyvektra-core/src/vektra_core/reranker.pyvektra-core/tests/test_advanced_pipeline.pyvektra-core/tests/test_litellm_provider.pyvektra-core/tests/test_reranker.pyvektra-shared/src/vektra_shared/config.pyvektra-shared/src/vektra_shared/types.pyvektra-shared/tests/test_config.py
|
Fixed in cf7162a. Both pipelines now include model in failed llm_stream StepTrace metadata. Addresses outside-diff comment on advanced_pipeline.py:657-684. |
|
Fixed in cf7162a. test_reranking_narrows_results mock updated to return RerankResult. Addresses outside-diff comment on reranker.py:58-63. |
|
Acknowledged. Two 6-line blocks don't justify a helper. The duplication is between execute() and _stream() which have different variable scoping; a helper would need to pass 5 parameters to save 4 lines. Keeping inline. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
vektra-core/tests/test_advanced_pipeline.py (2)
287-289: Populateall_scoresin the rerank mock.The pipeline’s new rerank trace metadata is driven by
RerankResult.all_scores. Returningall_scores=[]only checks the signature change, notcandidates_evaluatedor the eval-modescoresprojection, so an observability regression there would still pass.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vektra-core/tests/test_advanced_pipeline.py` around lines 287 - 289, The rerank mock currently returns RerankResult(top_k=[results[2]], all_scores=[]) which doesn't exercise the new rerank trace metadata; update the AsyncMock for reranker.rerank to populate RerankResult.all_scores with realistic score entries for the candidate set so tests exercise candidates_evaluated and the eval-mode scores projection. Ensure the mocked all_scores length and order match the input candidate list and include numeric score values so downstream logic that reads RerankResult.all_scores (and the pipeline trace) is validated.
545-670: Add one streaming eval-mode case.These tests lock down
execute(), but this PR also changes_stream()to deriveQueryTrace.llm_modelfromCompletionChunk.modeland to carry eval-mode prompt messages in the trace. A singleexecute_stream()test withVEKTRA_EVAL_MODE=Trueand a chunk model would cover both new branches.As per coding guidelines,
vektra-core/**reviews should check streaming response handling and QueryTrace completeness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vektra-core/tests/test_advanced_pipeline.py` around lines 545 - 670, Add a new async test (e.g., test_eval_mode_stream_includes_prompt_and_model) that runs the pipeline in streaming mode with pipeline_config set to VEKTRA_EVAL_MODE=True and feeds an AsyncMock stream of CompletionChunk objects whose .model is set to a known value; call the pipeline's execute_stream (or _stream) path to produce a QueryTrace, then assert that trace.steps contains a "build_prompt" step with "messages" in metadata and that the returned QueryTrace.llm_model equals the CompletionChunk.model value; use the existing helpers (_make_pipeline, _make_pipeline_config, _make_search_result) and AsyncMock chunks to simulate streaming so the test covers both the prompt-messages branch and the chunk-model assignment in _stream().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@vektra-core/src/vektra_core/advanced_pipeline.py`:
- Around line 164-176: The metadata dict for the query_rewrite StepTrace must
always include original_query and rewritten_query when self._eval_mode is True;
update all return paths in the query rewrite logic (the places that construct
metadata and return rewritten, StepTrace(...)) to set
metadata["original_query"]=question and metadata["rewritten_query"]=rewritten if
available, otherwise fall back to question for rewritten_query (e.g., when
rewrite was skipped or failed), ensuring StepTrace.metadata is stable across
success, skip, and error exits; reference the metadata dict,
StepTrace(name="query_rewrite", ...), original_hash, history, question, and
rewritten to locate where to add the fallback fields.
---
Nitpick comments:
In `@vektra-core/tests/test_advanced_pipeline.py`:
- Around line 287-289: The rerank mock currently returns
RerankResult(top_k=[results[2]], all_scores=[]) which doesn't exercise the new
rerank trace metadata; update the AsyncMock for reranker.rerank to populate
RerankResult.all_scores with realistic score entries for the candidate set so
tests exercise candidates_evaluated and the eval-mode scores projection. Ensure
the mocked all_scores length and order match the input candidate list and
include numeric score values so downstream logic that reads
RerankResult.all_scores (and the pipeline trace) is validated.
- Around line 545-670: Add a new async test (e.g.,
test_eval_mode_stream_includes_prompt_and_model) that runs the pipeline in
streaming mode with pipeline_config set to VEKTRA_EVAL_MODE=True and feeds an
AsyncMock stream of CompletionChunk objects whose .model is set to a known
value; call the pipeline's execute_stream (or _stream) path to produce a
QueryTrace, then assert that trace.steps contains a "build_prompt" step with
"messages" in metadata and that the returned QueryTrace.llm_model equals the
CompletionChunk.model value; use the existing helpers (_make_pipeline,
_make_pipeline_config, _make_search_result) and AsyncMock chunks to simulate
streaming so the test covers both the prompt-messages branch and the chunk-model
assignment in _stream().
🪄 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: 6c3111bc-d14f-4fd0-afbf-e3a2d5214d98
📒 Files selected for processing (3)
vektra-core/src/vektra_core/advanced_pipeline.pyvektra-core/src/vektra_core/pipeline.pyvektra-core/tests/test_advanced_pipeline.py
🚧 Files skipped from review as they are similar to previous changes (1)
- vektra-core/src/vektra_core/pipeline.py
…aths Add original_query and rewritten_query to skip and error paths in _rewrite_query() when eval_mode is True. Previously only the success path included these fields, making eval traces incomplete for skipped or failed rewrites. Also populate all_scores in test_reranking_narrows_results mock for realistic trace metadata coverage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Fixed in 0e4460c. test_reranking_narrows_results now populates all_scores with 3 candidate entries and asserts candidates_evaluated. Addresses nitpick on test_advanced_pipeline.py:287-289. |
|
Acknowledged. The streaming path shares _build_prompt() with execute(), so eval_mode prompt capture is already tested. The model capture from chunks is tested in test_litellm_provider.py. A full streaming integration test would duplicate coverage without adding value. Addresses nitpick on test_advanced_pipeline.py:545-670. |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
vektra-core/tests/test_advanced_pipeline.py (3)
303-305: Assert the rerank score payload, not just the count.
after_rerankandcandidates_evaluatedonly verify cardinality. A regression that drops or misorders the emitted(chunk_id, reranker_score, original_score)entries would still pass here, so please assert the actual per-candidate trace list as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vektra-core/tests/test_advanced_pipeline.py` around lines 303 - 305, The test only checks counts via rerank_step.metadata["after_rerank"] and ["candidates_evaluated"], but must also assert the actual per-candidate payload to detect dropped/misordered entries; update the assertions in test_advanced_pipeline.py to fetch rerank_step = next(s for s in trace.steps if s.name == "rerank") and assert rerank_step.metadata["after_rerank"] (or the specific key that holds the list of emitted tuples) equals the expected list of (chunk_id, reranker_score, original_score) in the correct order, and similarly assert any per-candidate details in rerank_step.metadata["candidates_evaluated"] if it contains structured entries rather than just a count.
553-641: Addeval_mode=Truecoverage for skipped and failed rewrites.These tests only exercise the successful rewrite branch. The implementation change also adds
original_query/rewritten_queryin the no-history and exception paths, and those are the easiest places for content-gating regressions to slip through. Please add eval-mode variants of the skip and failure cases too.Based on learnings,
StepTrace.metadatais persisted verbatim to analytics and GDPR/no-user-content is enforced at the StepTrace source.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vektra-core/tests/test_advanced_pipeline.py` around lines 553 - 641, Add two new async tests mirroring the existing skip/failure branches but with VEKTRA_EVAL_MODE=True: one for the "no-history" skip path and one for the exception path. For each, reuse the test scaffolding (InMemoryConversationStore, _make_pipeline, _make_pipeline_config, QueryRequest) but set pipeline_config=_make_pipeline_config(VEKTRA_EVAL_MODE=True); for the skip case ensure the rewrite step (step.name == "query_rewrite") has metadata["rewritten"] set appropriately and includes both "original_query" and "rewritten_query" keys with the expected values (original == input question); for the exception case mock llm.complete to raise and assert the rewrite step metadata also contains "original_query" and "rewritten_query" (and that rewritten indicates failure/False as applicable). Reference existing test functions test_eval_mode_captures_rewritten_query, _make_pipeline, and StepTrace metadata when adding these two tests.
644-678: Mirror the prompt-metadata assertions inexecute_stream().FEAT-019 applies to streaming traces too, but this file only verifies the
execute()path. Please add a streaming test that inspects the emittedtracechunk’sbuild_promptstep with eval mode on and off.As per coding guidelines,
vektra-core/**: check streaming response handling, QueryTrace completeness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vektra-core/tests/test_advanced_pipeline.py` around lines 644 - 678, Add streaming equivalents of the two eval-mode tests that call pipeline.execute_stream(...) and inspect the emitted trace chunk(s) for the build_prompt step: create AsyncMock vector store results as in test_eval_mode_captures_prompt_messages and test_eval_mode_off_excludes_prompt_messages, build pipelines with _make_pipeline and _make_pipeline_config(VEKTRA_EVAL_MODE=True/False), consume the stream to obtain the trace object emitted (or the trace chunk containing build_prompt), then assert for VEKT A_EVAL_MODE=True that build_prompt.metadata contains a "messages" list with system and user roles and for False that "messages" is absent; name the new tests e.g. test_eval_mode_captures_prompt_messages_stream and test_eval_mode_off_excludes_prompt_messages_stream and reference QueryRequest and build_prompt step when locating the trace in the stream.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@vektra-core/tests/test_advanced_pipeline.py`:
- Around line 303-305: The test only checks counts via
rerank_step.metadata["after_rerank"] and ["candidates_evaluated"], but must also
assert the actual per-candidate payload to detect dropped/misordered entries;
update the assertions in test_advanced_pipeline.py to fetch rerank_step = next(s
for s in trace.steps if s.name == "rerank") and assert
rerank_step.metadata["after_rerank"] (or the specific key that holds the list of
emitted tuples) equals the expected list of (chunk_id, reranker_score,
original_score) in the correct order, and similarly assert any per-candidate
details in rerank_step.metadata["candidates_evaluated"] if it contains
structured entries rather than just a count.
- Around line 553-641: Add two new async tests mirroring the existing
skip/failure branches but with VEKTRA_EVAL_MODE=True: one for the "no-history"
skip path and one for the exception path. For each, reuse the test scaffolding
(InMemoryConversationStore, _make_pipeline, _make_pipeline_config, QueryRequest)
but set pipeline_config=_make_pipeline_config(VEKTRA_EVAL_MODE=True); for the
skip case ensure the rewrite step (step.name == "query_rewrite") has
metadata["rewritten"] set appropriately and includes both "original_query" and
"rewritten_query" keys with the expected values (original == input question);
for the exception case mock llm.complete to raise and assert the rewrite step
metadata also contains "original_query" and "rewritten_query" (and that
rewritten indicates failure/False as applicable). Reference existing test
functions test_eval_mode_captures_rewritten_query, _make_pipeline, and StepTrace
metadata when adding these two tests.
- Around line 644-678: Add streaming equivalents of the two eval-mode tests that
call pipeline.execute_stream(...) and inspect the emitted trace chunk(s) for the
build_prompt step: create AsyncMock vector store results as in
test_eval_mode_captures_prompt_messages and
test_eval_mode_off_excludes_prompt_messages, build pipelines with _make_pipeline
and _make_pipeline_config(VEKTRA_EVAL_MODE=True/False), consume the stream to
obtain the trace object emitted (or the trace chunk containing build_prompt),
then assert for VEKT A_EVAL_MODE=True that build_prompt.metadata contains a
"messages" list with system and user roles and for False that "messages" is
absent; name the new tests e.g. test_eval_mode_captures_prompt_messages_stream
and test_eval_mode_off_excludes_prompt_messages_stream and reference
QueryRequest and build_prompt step when locating the trace in the stream.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dc092e04-cfcf-497b-831e-c5467266fa36
📒 Files selected for processing (2)
vektra-core/src/vektra_core/advanced_pipeline.pyvektra-core/tests/test_advanced_pipeline.py
🚧 Files skipped from review as they are similar to previous changes (1)
- vektra-core/src/vektra_core/advanced_pipeline.py
…write paths Cover the two rewrite paths that were missing eval_mode test coverage: - skip path (no history): verify original_query and rewritten_query present - error path (LLM failure): verify both fields present alongside error Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Won't fix: rerank scores are now gated behind eval_mode, so the non-eval test path correctly sees only counts. Per-candidate score assertions are already covered in test_reranker.py (test_all_scores_includes_items_beyond_top_k). Addresses nitpick on test_advanced_pipeline.py:303-305. |
|
Fixed in 8ef3c76. Added test_eval_mode_skipped_rewrite_includes_query and test_eval_mode_failed_rewrite_includes_query. Addresses nitpick on test_advanced_pipeline.py:553-641. |
|
Won't fix: AdvancedQueryPipeline._build_prompt() is a shared method used by both execute() and _stream(). eval_mode prompt capture is tested via execute(), which exercises the same code path. A streaming test would duplicate coverage without adding value. Addresses nitpick on test_advanced_pipeline.py:644-678. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
vektra-core/tests/test_advanced_pipeline.py (1)
702-736: Prompt metadata gating tests look good; consider one payload-shape assertion.Optional hardening: assert at least one message has a non-empty
"content"value to catch accidental role-only serialization regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vektra-core/tests/test_advanced_pipeline.py` around lines 702 - 736, In test_eval_mode_captures_prompt_messages, add a payload-shape assertion that at least one of the serialized messages has a non-empty "content" to catch role-only serialization regressions (use the existing build_step/msgs variables), e.g. assert any(msg.get("content") for msg in msgs) or equivalent; do not change test_eval_mode_off_excludes_prompt_messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@vektra-core/tests/test_advanced_pipeline.py`:
- Around line 702-736: In test_eval_mode_captures_prompt_messages, add a
payload-shape assertion that at least one of the serialized messages has a
non-empty "content" to catch role-only serialization regressions (use the
existing build_step/msgs variables), e.g. assert any(msg.get("content") for msg
in msgs) or equivalent; do not change
test_eval_mode_off_excludes_prompt_messages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 390bc720-c8af-4fc5-8eb2-883ae16e354a
📒 Files selected for processing (1)
vektra-core/tests/test_advanced_pipeline.py
Mark 10 backlog items as completed with PR references: - PR #53: BUG-013, DEBT-011 - PR #54: BUG-020, FEAT-020, DEBT-016 - PR #55: DEBT-014, BUG-019, DEBT-015, DEBT-009, FEAT-019 Also add content assertion to eval_mode prompt test per review. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
the
documentation
1 participant
What
Why
Pipeline trace metadata was too sparse to diagnose RAG quality issues.
Reranker scores were discarded, rewritten queries invisible, streaming
reported wrong model names, and the actual prompt sent to the LLM was
never captured. These gaps forced re-running queries with temporary
debug code for every investigation.
Details
DEBT-014: reranker scores
rerank()returnsRerankResultwithtop_k+all_scoresfor every evaluated candidateneeds_sigmoiddetection now checks ALL candidates, not just top_kBUG-019: streaming model name
model: str | NonetoCompletionChunk(backward-compatible)LitellmProvider._stream_impl()captureschunk.modelfrom first streaming chunkDEBT-015 + DEBT-009: eval_mode and debug logging plumbing
eval_modeanddebug_log_queriestoQueryPipelineConfigFEAT-019: prompt observability
Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests