Conversation
- Optional enrollment mode for LMS integrations (FEAT-003) - Widget UX: collapsible sources, markdown-ready citations, a11y improvements - Sparse embedding generation during ingest (BUG-011), conversation auto-ID (BUG-010) - Registry None guard in health check, sparse embedding count validation - Combo D RAG tuning defaults, OpenAI-compatible API base, parameterized ports Reviews: 16/16 addressed (CodeRabbit 12, Gemini 1, manual 3) Tests: 578 passed Refs: FEAT-003, BUG-010, BUG-011, DEBT-002, DEBT-003, DEBT-008
- Add CHANGELOG entries for v0.2.0 (retroactive) and v0.3.0 - Bump all pyproject.toml and __version__ from 0.2.0-dev to 0.3.0 - Add commit signing setup guide to CONTRIBUTING.md Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use $(git config user.email) in allowed_signers example (Gemini) - Hyphenate "rate-limiting middleware" in CHANGELOG (CodeRabbit) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- README: Phase 2 status "Planned" -> "Complete" - CONTRIBUTING: test command includes analytics/learn, module boundaries updated from 5 to 8 components - GOVERNANCE: Phase 2 transition criteria updated - configuration.md: fix 6 stale defaults (Combo D), add 18 new Phase 2 variables (rewrite, rerank, webhook, qdrant, learn, pii) - error-codes.md: add ERR-LEARN-001 through 004 - .env.example: add Phase 2 variable sections Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add phase qualifiers to security vars (safeguard, multi_tenant, conversation_key) - Add "Phase 2 only" to analytics_retention_days - Fix variable count: 53 VEKTRA_* across VektraSettings + Phase 2 sub-configs - Clarify that Phase 2 sub-configs are validated independently Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Development phases are internal process milestones, not user-facing distinctions. All features are available, no need to label their origin. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Bump version 0.2.0-dev to 0.3.0 (8 pyproject.toml + 3 __init__.py) - Add CHANGELOG entries for v0.2.0 (retroactive) and v0.3.0 - Update docs: 6 stale defaults fixed, 18 new config vars, ERR-LEARN codes - Add commit signing guide to CONTRIBUTING.md - Update README/GOVERNANCE/CONTRIBUTING for Phase 2 completion Reviews: 5/5 addressed (CodeRabbit 3, Gemini 2) Tests: 578 passed
fvadicamo
added
documentation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughConfiguration defaults and environment surface updated for v0.3.0 (LLM api_base, query rewrite/rerank, webhooks, PII threshold, ingest table/parent-child), package version bumps, learn enrollment/namespace behavior and conversation auto-generation, sparse embeddings support, safeguards fallback, health-tier separation, widget UI/behavior changes, and docker-compose port parameterization. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
🚥 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)
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 |
github-actions
Bot
added
component:shared
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on stabilizing and improving the e-learning vertical and widget, addressing bugs, and updating configurations for better performance and usability. It prepares the e-learning vertical and widget for pilot deployment by completing Sprint 1 of the post-Phase 2 roadmap. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![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 prepares for the v0.3.0 release, incorporating a wide range of features, bug fixes, and documentation updates from sprint 1. Key changes include implementing optional enrollment for LMS integrations, adding sparse embedding generation to the ingest pipeline, and improving the chatbot widget's UX with collapsible sources. My review focuses on improving code maintainability. I've suggested refactoring duplicated validation logic in the ingest pipeline and extracting complex namespace resolution logic in the learn API into a helper function to improve readability, noting that the latter suggestion also reinforces the rule for explicit JWT claim validation. Overall, the changes are well-implemented and tested, reflecting a solid release.
Main had commits from release/v0.2.0 (PR #38) that were not back-merged into develop. All conflicts resolved keeping develop versions (v0.3.0 > v0.2.0). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The merge of main into develop auto-resolved safeguards/__init__.py incorrectly, taking main's raise-ValueError approach over develop's graceful-fallback approach (try/except with passthrough fallback). Restores the develop behavior: unknown modes log a warning and fall back to passthrough, presidio import failures fall back gracefully. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- scripts/ingest.sh: add .md content-type mapping (text/markdown) - audit_rows.html: use isoformat() to preserve timezone in cursor - qdrant.py: fix gRPC deadline check with "in" instead of exact match Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mirror the execute() recheck in _stream(): if post_retrieval safeguard filtered all chunks, set no_relevant_context and return early instead of calling LLM with empty context. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- middleware.py: hoist _helper_segments to module-level _HELPER_TAILS - test_rls.py: add conflicting body to GET query param precedence test - qdrant.py: add debug log on race condition resolution - ui.py: narrow IntegrityError to unique violation (pgcode 23505) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
POST /api-keys/revoke now produces "revoke_api-keys" instead of "create_api-keys". The path tail (/create, /revoke, /delete, /update) overrides the HTTP method verb when present. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Version 1.10 (2026-03-01, OQ-018/OQ-019 resolution) was listed after Version 2.0 (2026-03-14) due to merge order. Moved to correct position. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Sync main into develop (diverged history from v0.2.0 release) - Restore safeguard graceful fallback lost in auto-merge - Fix 3 critical bugs: ingest.sh .md mapping, audit cursor timezone, gRPC deadline check - Add streaming post_retrieval recheck, audit verb from helper tail - Hoist _HELPER_TAILS, narrow IntegrityError, qdrant race log, test improvements Reviews: 12/12 addressed (CodeRabbit 11, Gemini 1) Tests: 576 passed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
vektra-core/src/vektra_core/safeguards/__init__.py (1)
27-43: Normalize safeguard mode before branching.Current matching is exact; values like
"Presidio"or" presidio "silently degrade to passthrough. Normalizing once makes config handling safer.♻️ Suggested refactor
def create_safeguard(mode: str, *, pii_chunk_threshold: int = 3) -> SafeguardHook: @@ - if mode == "presidio": + normalized_mode = mode.strip().lower() + + if normalized_mode == "presidio": try: from vektra_core.safeguards.presidio import PresidioPIISafeguard return PresidioPIISafeguard(pii_chunk_threshold=pii_chunk_threshold) @@ - if mode != "passthrough": + if normalized_mode != "passthrough": log.warning("safeguard_unknown_mode", mode=mode, fallback="passthrough")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vektra-core/src/vektra_core/safeguards/__init__.py` around lines 27 - 43, Normalize the incoming mode value before the branching logic by assigning a cleaned version (e.g., mode = mode.strip().lower() if mode is a string) and then use that normalized variable for the comparisons; update the checks that currently compare mode == "presidio" and mode != "passthrough" to use the normalized mode so inputs like "Presidio" or " presidio " correctly match and still fall back to returning PassthroughSafeguard() and logging via log.warning when appropriate, while leaving references to PresidioPIISafeguard and PassthroughSafeguard unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/ingest.sh`:
- Line 65: The CLI help/usage text was not updated to reflect the new .md case
(you added the md) pattern that sets CONTENT_TYPE="text/markdown"); update the
script's help/usage output (the function or block that prints supported formats
/ examples) to include Markdown and .md alongside PDF, DOCX, PPTX so the usage
string matches the md) => CONTENT_TYPE="text/markdown" handling.
In `@vektra-learn/src/vektra_learn/service.py`:
- Around line 53-57: The TokenRequest model's namespace Field currently allows
empty strings; update the Field on namespace in TokenRequest to forbid empty
values by adding a minimum length constraint (e.g., min_length=1) or use a
constrained type (e.g., constr(min_length=1)) and optionally strip whitespace,
so that None is still allowed but "" (or only-whitespace) is rejected and will
not produce an empty JWT claim.
---
Nitpick comments:
In `@vektra-core/src/vektra_core/safeguards/__init__.py`:
- Around line 27-43: Normalize the incoming mode value before the branching
logic by assigning a cleaned version (e.g., mode = mode.strip().lower() if mode
is a string) and then use that normalized variable for the comparisons; update
the checks that currently compare mode == "presidio" and mode != "passthrough"
to use the normalized mode so inputs like "Presidio" or " presidio " correctly
match and still fall back to returning PassthroughSafeguard() and logging via
log.warning when appropriate, while leaving references to PresidioPIISafeguard
and PassthroughSafeguard unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5644fd6c-87a1-4be0-8cd3-ad54662eed2b
⛔ Files ignored due to path filters (3)
.s2s/BACKLOG.mdis excluded by!.s2s/**.s2s/architecture.mdis excluded by!.s2s/**uv.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (46)
.env.example.gitignoreCHANGELOG.mdCONTRIBUTING.mdGOVERNANCE.mdREADME.mddocker-compose.ymldocs/reference/configuration.mddocs/reference/error-codes.mdscripts/ingest.shvektra-admin/pyproject.tomlvektra-admin/src/vektra_admin/health.pyvektra-admin/src/vektra_admin/middleware.pyvektra-admin/src/vektra_admin/templates/partials/audit_rows.htmlvektra-admin/src/vektra_admin/ui.pyvektra-admin/tests/test_health.pyvektra-admin/tests/test_rls.pyvektra-analytics/pyproject.tomlvektra-analytics/src/vektra_analytics/__init__.pyvektra-app/pyproject.tomlvektra-app/src/vektra_app/__init__.pyvektra-app/src/vektra_app/main.pyvektra-core/pyproject.tomlvektra-core/src/vektra_core/pipeline.pyvektra-core/src/vektra_core/providers/litellm_provider.pyvektra-core/src/vektra_core/safeguards/__init__.pyvektra-core/tests/test_safeguards.pyvektra-index/pyproject.tomlvektra-index/src/vektra_index/providers/qdrant.pyvektra-ingest/pyproject.tomlvektra-ingest/src/vektra_ingest/pipeline.pyvektra-ingest/tests/test_pipeline.pyvektra-ingest/tests/test_versioning.pyvektra-learn/pyproject.tomlvektra-learn/src/vektra_learn/api.pyvektra-learn/src/vektra_learn/service.pyvektra-learn/tests/test_api.pyvektra-learn/tests/test_service.pyvektra-learn/widget/src/api-client.jsvektra-learn/widget/src/chat-ui.jsvektra-learn/widget/src/index.jsvektra-learn/widget/src/styles.jsvektra-shared/pyproject.tomlvektra-shared/src/vektra_shared/__init__.pyvektra-shared/src/vektra_shared/config.pyvektra-shared/tests/test_config.py
- service.py: disallow empty namespace in TokenRequest (min_length=1) - ingest.sh: update help text to include MD format - safeguards: normalize mode with strip().lower() before matching Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Fixed in PR #46: normalized safeguard mode with strip().lower() (CodeRabbit nitpick on safeguards/init.py:27-43). |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Disallow empty namespace in TokenRequest (min_length=1) - Update ingest.sh help text and add .markdown extension support - Normalize safeguard mode with strip().lower() Reviews: 2/2 addressed (CodeRabbit 1, Gemini 1) Tests: 576 passed Refs: PR #44 review comments
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-learn/src/vektra_learn/service.py`:
- Around line 168-175: The audit event "dashboard_token_generated" currently
omits the effective namespace even when a scoped token is created; update the
code that builds and emits the audit event (the same area that constructs
payload and uses req.namespace) to include the JSON field "namespace" with the
value from req.namespace when present so the emitted event carries the
business-facing "namespace" key for traceability; ensure you modify the event
emission logic that references payload/dashboard_token_generated so the audit
payload matches the token payload's namespace.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9ecc5f1a-6706-4746-b913-01029fe82ed5
📒 Files selected for processing (3)
scripts/ingest.shvektra-core/src/vektra_core/safeguards/__init__.pyvektra-learn/src/vektra_learn/service.py
✅ Files skipped from review due to trivial changes (1)
- scripts/ingest.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- vektra-core/src/vektra_core/safeguards/init.py
Add effective namespace (req.namespace or course_id fallback) to the dashboard_token_generated audit log for scoped access traceability. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add effective namespace to dashboard_token_generated audit event Reviews: 1/1 addressed (Gemini: DashboardTokenOrm column deferred) Tests: 576 passed Refs: PR #44 review comment
This was referenced Mar 25, 2026
4 tasks
This was referenced Apr 21, 2026
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.
What
This release includes:
Why
Completes Sprint 1 of the post-Phase 2 roadmap. Stabilizes the e-learning vertical and widget for pilot deployment.
Details
PR #42: feat(learn): optional enrollment for LMS integrations
PR #43: chore: prepare v0.3.0 release
Testing
Checklist
Summary by CodeRabbit
New Features
Improvements
Documentation