LCORE-3496: add file processor to default config - #2402
Merged
Conversation
Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
Contributor
WalkthroughThe default baseline now enables the ChangesFile processor baseline
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
✨ Simplify code
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}
Contributor
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 `@tests/unit/test_llama_stack_synthesize.py`:
- Around line 126-134: Update
test_load_default_baseline_includes_file_processors to locate the pypdf provider
entry and assert its default_chunk_size_tokens is 800 and
default_chunk_overlap_tokens is 400, while preserving the existing provider_id
and provider_type checks.
🪄 Autofix
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: ASSERTIVE
Plan: Pro Plus
Run ID: 5a22cc09-53b8-469f-b524-b033c068d06b
📒 Files selected for processing (2)
src/data/default_run.yamltests/unit/test_llama_stack_synthesize.py
📜 Review details
⏰ Context from checks skipped due to timeout. (13)
- GitHub Check: unit_tests (3.12)
- GitHub Check: build-pr
- GitHub Check: unit_tests (3.13)
- GitHub Check: integration_tests (3.12)
- GitHub Check: integration_tests (3.13)
- GitHub Check: Pylinter
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E Tests for Lightspeed Evaluation job
⚠️ CI failures not shown inline (2)
GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-8-on-pull-request: Failed
Conclusion: failure
Konflux kflux-prd-rh02/lightspeed-stack-0-8-on-pull-request has <b>failed</b>.
<ul>
<li><b>Namespace</b>: <a href="https://konflux-ui.apps.kflux-prd-rh02.0fk9.p1.openshiftapps.com/ns/lightspeed-core-tenant">lightspeed-core-tenant</a></li>
<li><b>PipelineRun:</b> <a href="https://konflux-ui.apps.kflux-prd-rh02.0fk9.p1.openshiftapps.com/ns/lightspeed-core-tenant/pipelinerun/lightspeed-stack-0-8-on-pull-request-z4d8t">lightspeed-stack-0-8-on-pull-request-z4d8t</a></li>
</ul>
<hr>
<h4>Task Statuses:</h4>
<table>
<tr><th>Status</th><th>Duration</th><th>Name</th></tr>
<tr>
<td>🟢 Succeeded</td>
<td>4 seconds</td><td>
[init](https://konflux-ui.apps.kflux-prd-rh02.0fk9.p1.openshiftapps.com/ns/lightspeed-core-tenant/pipelinerun/lightspeed-stack-0-8-on-pull-request-z4d8t/logs/init)
</td></tr>
<tr>
<td>🟢 Succeeded</td>
<td>15 seconds</td><td>
[clone-repository](https://konflux-ui.apps.kflux-prd-rh02.0fk9.p1.openshiftapps.com/ns/lightspeed-core-tenant/pipelinerun/lightspeed-stack-0-8-on-pull-request-z4d8t/logs/clone-repository)
</td></tr>
<tr>
<td>🟢 Succeeded</td>
<td>10 seconds</td><td>
[prefetch-dependencies](https://konflux-ui.apps.kflux-prd-rh02.0fk9.p1.openshiftapps.com/ns/lightspeed-core-tenant/pipelinerun/lightspeed-stack-0-8-on-pull-request-z4d8t/logs/prefetch-dependencies)
</td></tr>
<tr>
<td>🔴 Failed</td>
<td>11 seconds</td><td>
[build-images](https://konflux-ui.apps.kflux-prd-rh02.0fk9.p1.openshiftapps.com/ns/lightspeed-core-tenant/pipelinerun/lightspeed-stack-0-8-on-pull-request-z4d8t/logs/build-images)
</td></tr>
</table>
GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request: Failed
Conclusion: failure
Konflux kflux-prd-rh02/lightspeed-stack-0-7-on-pull-request has <b>failed</b>.
<ul>
<li><b>Namespace</b>: <a href="https://konflux-ui.apps.kflux-prd-rh02.0fk9.p1.openshiftapps.com/ns/lightspeed-core-tenant">lightspeed-core-tenant</a></li>
<li><b>PipelineRun:</b> <a href="https://konflux-ui.apps.kflux-prd-rh02.0fk9.p1.openshiftapps.com/ns/lightspeed-core-tenant/pipelinerun/lightspeed-stack-0-7-on-pull-request-xsqsg">lightspeed-stack-0-7-on-pull-request-xsqsg</a></li>
</ul>
<hr>
<h4>Task Statuses:</h4>
<table>
<tr><th>Status</th><th>Duration</th><th>Name</th></tr>
<tr>
<td>🟢 Succeeded</td>
<td>4 seconds</td><td>
[init](https://konflux-ui.apps.kflux-prd-rh02.0fk9.p1.openshiftapps.com/ns/lightspeed-core-tenant/pipelinerun/lightspeed-stack-0-7-on-pull-request-xsqsg/logs/init)
</td></tr>
<tr>
<td>🟢 Succeeded</td>
<td>15 seconds</td><td>
[clone-repository](https://konflux-ui.apps.kflux-prd-rh02.0fk9.p1.openshiftapps.com/ns/lightspeed-core-tenant/pipelinerun/lightspeed-stack-0-7-on-pull-request-xsqsg/logs/clone-repository)
</td></tr>
<tr>
<td>🔴 Failed</td>
<td>20 seconds</td><td>
[prefetch-dependencies](https://konflux-ui.apps.kflux-prd-rh02.0fk9.p1.openshiftapps.com/ns/lightspeed-core-tenant/pipelinerun/lightspeed-stack-0-7-on-pull-request-xsqsg/logs/prefetch-dependencies)
</td></tr>
</table>
🧰 Additional context used
📓 Path-based instructions (2)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.
Files:
tests/unit/test_llama_stack_synthesize.pysrc/data/default_run.yaml
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use pytest for unit tests, shared fixtures in
conftest.py,pytest-mockfor mocks,pytest.mark.asynciofor async tests, and maintain at least 60% unit-test coverage.
Files:
tests/unit/test_llama_stack_synthesize.py
🧠 Learnings (2)
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
Applied to files:
tests/unit/test_llama_stack_synthesize.py
📚 Learning: 2026-05-20T08:09:30.641Z
Learnt from: max-svistunov
Repo: lightspeed-core/lightspeed-stack PR: 1580
File: docs/design/llama-stack-config-merge/poc-results/library-mode/synthesized-run.yaml:107-110
Timestamp: 2026-05-20T08:09:30.641Z
Learning: In Llama-stack config YAMLs, when defining a Llama Guard safety shield entry, set `provider_shield_id` to the *guard model identifier* (e.g., `meta-llama/Llama-Guard-3-8B`). Do not use a chat/generative model id (e.g., `openai/gpt-4o-mini`): a chat-model id (or `native_override`) indicates only an override landed and does **not** mean the safety shield is actually gating queries. Ensure any E2E coverage for the related implementation (JIRA/E2E tests) exercises a real Llama Guard model to verify that the shield is effective.
Applied to files:
src/data/default_run.yaml
🔇 Additional comments (1)
src/data/default_run.yaml (1)
11-17: LGTM!Also applies to: 28-28, 53-59
Comment on lines
+126
to
+134
| def test_load_default_baseline_includes_file_processors() -> None: | ||
| """Default stack ships file_processors so vector-store file attach works.""" | ||
| baseline = load_default_baseline() | ||
| assert "file_processors" in baseline["apis"] | ||
| processors = baseline["providers"]["file_processors"] | ||
| assert any( | ||
| p.get("provider_id") == "pypdf" and p.get("provider_type") == "inline::pypdf" | ||
| for p in processors | ||
| ) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the configured chunk values.
The test passes when pypdf has the correct provider type but incorrect chunking values. Assert default_chunk_size_tokens == 800 and default_chunk_overlap_tokens == 400 to cover the configuration added on Lines 57-59 of src/data/default_run.yaml.
Proposed test update
processors = baseline["providers"]["file_processors"]
- assert any(
- p.get("provider_id") == "pypdf" and p.get("provider_type") == "inline::pypdf"
- for p in processors
+ pypdf = next(
+ (p for p in processors if p.get("provider_id") == "pypdf"),
+ None,
)
+ assert pypdf is not None
+ assert pypdf["provider_type"] == "inline::pypdf"
+ assert pypdf["config"]["default_chunk_size_tokens"] == 800
+ assert pypdf["config"]["default_chunk_overlap_tokens"] == 400📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def test_load_default_baseline_includes_file_processors() -> None: | |
| """Default stack ships file_processors so vector-store file attach works.""" | |
| baseline = load_default_baseline() | |
| assert "file_processors" in baseline["apis"] | |
| processors = baseline["providers"]["file_processors"] | |
| assert any( | |
| p.get("provider_id") == "pypdf" and p.get("provider_type") == "inline::pypdf" | |
| for p in processors | |
| ) | |
| def test_load_default_baseline_includes_file_processors() -> None: | |
| """Default stack ships file_processors so vector-store file attach works.""" | |
| baseline = load_default_baseline() | |
| assert "file_processors" in baseline["apis"] | |
| processors = baseline["providers"]["file_processors"] | |
| pypdf = next( | |
| (p for p in processors if p.get("provider_id") == "pypdf"), | |
| None, | |
| ) | |
| assert pypdf is not None | |
| assert pypdf["provider_type"] == "inline::pypdf" | |
| assert pypdf["config"]["default_chunk_size_tokens"] == 800 | |
| assert pypdf["config"]["default_chunk_overlap_tokens"] == 400 |
🤖 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/unit/test_llama_stack_synthesize.py` around lines 126 - 134, Update
test_load_default_baseline_includes_file_processors to locate the pypdf provider
entry and assert its default_chunk_size_tokens is 800 and
default_chunk_overlap_tokens is 400, while preserving the existing provider_id
and provider_type checks.
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
2 participants
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.
Description
file_processorsOGX API and the built-inpypdfprocessor to thedefault_run.yamlbase for unified configType of change
pyproject.toml+uv.lock]requirements.*.txtfor Konflux]Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
New Features
Tests