Repository navigation
refactor(export): optimize shared task context queries and remove dead code - #2198
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR excludes knowledge-base contexts from public shared-task responses and PDF exports, batch-loads remaining contexts, removes PDF knowledge-base rendering and related UI types, and adds coverage for attachment-only serialization. It also adds explicit ChangesContext and export behavior
Explicit return annotations
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
…t-from-public-outputs
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/app/services/export/docx_generator.py (1)
151-151: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
-> Nonereturn annotations to new functions.
backend/app/services/export/docx_generator.py#L151-L151: annotate_add_document_header.backend/tests/services/test_docx_generator_fonts.py#L32-L32: annotatetest_normal_style_defines_latin_and_east_asian_fonts.backend/tests/services/test_docx_generator_fonts.py#L50-L50: annotatetest_inline_code_uses_monospace_latin_and_document_cjk_font.backend/tests/services/test_docx_generator_prompt_sanitization.py#L77-L77: annotatetest_knowledge_base_context_not_written_to_document.As per coding guidelines, Python code must use type hints.
🤖 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 `@backend/app/services/export/docx_generator.py` at line 151, Add explicit -> None return annotations to _add_document_header and the three listed test functions: test_normal_style_defines_latin_and_east_asian_fonts and test_inline_code_uses_monospace_latin_and_document_cjk_font in backend/tests/services/test_docx_generator_fonts.py, and test_knowledge_base_context_not_written_to_document in backend/tests/services/test_docx_generator_prompt_sanitization.py.Source: Coding guidelines
backend/app/services/shared_task.py (1)
828-838: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider batching the per-subtask context query.
This query runs once per subtask inside the loop (N+1), on a public/unauthenticated endpoint. The function already batches user lookups via
sender_ids/user_name_mapa few lines above (Lines 815-825) — the same pattern (singleINquery + group bysubtask_id) could be applied here to avoid repeated round-trips as subtask counts grow.♻️ Sketch of batched context fetch
+ subtask_ids = [sub.id for sub in subtasks] + contexts_by_subtask: dict[int, list] = {} + if subtask_ids: + all_contexts = ( + db.query(SubtaskContext) + .filter( + SubtaskContext.subtask_id.in_(subtask_ids), + SubtaskContext.context_type != ContextType.KNOWLEDGE_BASE.value, + ) + .all() + ) + for ctx in all_contexts: + contexts_by_subtask.setdefault(ctx.subtask_id, []).append(ctx) + for sub in subtasks: - # Knowledge base contexts are runtime metadata and are not public. - contexts = ( - db.query(SubtaskContext) - .filter( - SubtaskContext.subtask_id == sub.id, - SubtaskContext.context_type != ContextType.KNOWLEDGE_BASE.value, - ) - .all() - ) + contexts = contexts_by_subtask.get(sub.id, [])🤖 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 `@backend/app/services/shared_task.py` around lines 828 - 838, Replace the per-subtask SubtaskContext query inside the public_subtasks loop with one batched query using all subtask IDs and the existing knowledge-base exclusion, then group the fetched contexts by subtask_id and reuse that mapping while constructing public_subtasks. Preserve the current behavior for subtasks with no matching contexts.
🤖 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 `@backend/tests/services/test_shared_task_prompt_sanitization.py`:
- Around line 121-174: Update
test_keeps_attachments_and_filters_knowledge_base_contexts so the SubtaskContext
records reference a persisted Subtask row, using the mock subtask’s actual
persisted identifier when available. Ensure the test database flush succeeds
with foreign-key enforcement enabled while preserving the existing attachment
and knowledge-base context assertions.
---
Nitpick comments:
In `@backend/app/services/export/docx_generator.py`:
- Line 151: Add explicit -> None return annotations to _add_document_header and
the three listed test functions:
test_normal_style_defines_latin_and_east_asian_fonts and
test_inline_code_uses_monospace_latin_and_document_cjk_font in
backend/tests/services/test_docx_generator_fonts.py, and
test_knowledge_base_context_not_written_to_document in
backend/tests/services/test_docx_generator_prompt_sanitization.py.
In `@backend/app/services/shared_task.py`:
- Around line 828-838: Replace the per-subtask SubtaskContext query inside the
public_subtasks loop with one batched query using all subtask IDs and the
existing knowledge-base exclusion, then group the fetched contexts by subtask_id
and reuse that mapping while constructing public_subtasks. Preserve the current
behavior for subtasks with no matching contexts.
🪄 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: 36689d34-62ed-4620-b28b-f580e4d79cbb
📒 Files selected for processing (12)
backend/app/services/export/docx_generator.pybackend/app/services/shared_task.pybackend/tests/services/test_docx_generator_fonts.pybackend/tests/services/test_docx_generator_prompt_sanitization.pybackend/tests/services/test_shared_task_prompt_sanitization.pyfrontend/src/features/tasks/components/message/MessagesArea.tsxfrontend/src/features/tasks/components/share/ExportSelectModal.tsxfrontend/src/i18n/locales/en/chat.jsonfrontend/src/i18n/locales/zh-CN/chat.jsonfrontend/src/utils/pdf/index.tsfrontend/src/utils/pdf/renderers/message.tsfrontend/src/utils/pdf/types.ts
💤 Files with no reviewable changes (4)
- frontend/src/utils/pdf/index.ts
- frontend/src/i18n/locales/en/chat.json
- frontend/src/i18n/locales/zh-CN/chat.json
- frontend/src/utils/pdf/types.ts
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 `@backend/app/services/shared_task.py`:
- Line 827: Update the type annotation for contexts_by_subtask_id in the
surrounding task-processing function so Dict is defined before use: import Dict
from typing, or replace it with the supported built-in generic syntax. Ensure
the module no longer triggers Ruff F821.
🪄 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: 30eeff89-1797-464b-9fc9-73253da5adaf
📒 Files selected for processing (5)
backend/app/services/export/docx_generator.pybackend/app/services/shared_task.pybackend/tests/services/test_docx_generator_fonts.pybackend/tests/services/test_docx_generator_prompt_sanitization.pybackend/tests/services/test_shared_task_prompt_sanitization.py
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/tests/services/test_docx_generator_fonts.py
- backend/tests/services/test_shared_task_prompt_sanitization.py
…d code (wecode-ai#2198) * fix(export): 隐藏导出和公开分享中的知识库上下文 * fix(export): 统一 DOCX 中英文导出字体 * refactor(export): 清理 PDF 知识库导出死代码 * fix(export): 完善公开分享知识库上下文过滤 * refactor(export): 优化公开分享上下文查询 * fix(export): 修复公开分享上下文类型注解 --------- Co-authored-by: axb <qindi@staff.weibo.com>
Resolve conflicts between the DingTalk external-knowledge feature and upstream capability/permission refactors (wecode-ai#2311, wecode-ai#2348, wecode-ai#2285, wecode-ai#2198). Co-Authored-By: Claude <noreply@anthropic.com>
Summary by CodeRabbit
Changes
Improvements
Tests