docs(backlog): track v0.5.0 release-PR review feedback#72
Conversation
Two new DEBT entries from Gemini review on the develop->main release PR (#71). All medium/low severity, deferred from v0.5.0 release per zero-new-commit policy on release PRs: - DEBT-020 (medium): audit log skipped when request_id is missing on admin writes. patch_namespace_config and similar endpoints should synthesize a fallback uuid4 so NFR-007 holds even if the request-id middleware misbehaves. - DEBT-021 (low): _fetch_document_names adds a separate Postgres round-trip per query. The WI-2 plan called for a JOIN with source_documents inside the existing chunk-fetch query; v0.5.0 opted for a clarity-first separate call, consolidation tracked here. Three other Gemini comments on the same review map to existing items (DEBT-018 for :root scope) or were false positives (claimed missing imports already present at line 13). The four code-quality bot LOW nitpicks ("self_inner" in nested test fakes) are style noise on disambiguating closure-bound mocks; no entry created.
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request adds two new technical debt entries to the backlog: DEBT-020, which addresses missing audit logs when a request ID is absent, and DEBT-021, which aims to optimize document name retrieval. Feedback suggests broadening the scope of DEBT-020 to include all sensitive endpoints rather than just admin writes to ensure comprehensive audit coverage. Additionally, for DEBT-021, the proposed optimization should be generalized to support non-SQL providers like Qdrant, ensuring consistency with the project's multi-provider architecture.
Address two Gemini review comments on PR #72: - DEBT-020 (3143979031): title "on admin writes" too narrow per NFR-007 ("audit every sensitive content access"). Sensitive read endpoints like `get_conversation_turns` (vektra-admin/api.py:491-539) share the same `if request_id:` pattern. Renamed to "on sensitive endpoints" and expanded the sweep + acceptance criteria to cover both reads and writes. - DEBT-021 (3143979036): "Single Postgres query" was Pgvector-centric and ignored the multi-provider architecture. Reframed as "provider-aligned filename resolution" with two explicit paths: (A) Postgres-side JOIN — provider-agnostic since chunks live in document_chunks for both pgvector and qdrant deployments, (B) Qdrant-native payload denormalization — follow-up for Qdrant-heavy deployments, separate ingest-side change.
Summary
Two new BACKLOG entries from the Gemini review on the v0.5.0 release PR (#71).
All medium/low severity, deferred from v0.5.0 per zero-new-commit policy on
release PRs.
New entries
request_idis missing on admin writes.patch_namespace_configand similar endpoints should synthesize a fallbackuuid4()so NFR-007 holds even if the request-id middleware misbehaves._fetch_document_namesadds a separate Postgres round-trip per query. The WI-2 plan called for a JOIN withsource_documentsinside the existing chunk-fetch query; v0.5.0 opted for a clarity-first separate call, consolidation tracked here.Other Gemini feedback (not new entries)
:rootinjection scope (chat-ui.js:145)UTC, datetimeimports (api.py:32)self_innerin nested test fakes (test_pipeline.py x4)Test plan
Refs