feat(backend): add subtask KB sync to task level with priority rules #845
Conversation
Implement automatic synchronization of subtask-level knowledge base selections to task-level knowledgeBaseRefs, enabling KB reuse across all subtasks within a task. Changes: - Add sync_subtask_kb_to_task method in TaskKnowledgeBaseService for automatic KB sync with deduplication and limit enforcement - Modify link_contexts_to_subtask to trigger KB sync when creating knowledge base contexts - Implement KB priority logic: subtask-level KB > task-level KB - Remove group chat restriction for task-level KB binding - Add comprehensive unit tests for all new functionality Key features: - Append mode with deduplication by name + namespace - Silent skip when KB limit (10) is reached - Permission check before syncing (no-access KBs are skipped) - Works for both group chat and non-group chat tasks
📝 WalkthroughWalkthroughLinking chat contexts to a subtask now accepts Changes
Sequence DiagramsequenceDiagram
participant Client
participant ChatNS as Chat Namespace API
participant ContextSvc as Context Preprocessing
participant TaskKB as Task KB Service
participant DB as Database
Client->>ChatNS: on_chat_send(subtask_id, user_id, contexts, task, user_name)
ChatNS->>ContextSvc: link_contexts_to_subtask(subtask_id, user_id, contexts, task, user_name)
rect rgb(240,248,255)
Note over ContextSvc,DB: create/link subtask contexts & attachments
ContextSvc->>DB: persist SubtaskContext/attachments
end
rect rgb(240,255,240)
Note over ContextSvc: determine KB ids (priority: subtask KBs -> task-bound KBs)
alt subtask KBs exist
ContextSvc->>ContextSvc: select subtask KB ids
else
ContextSvc->>ContextSvc: call _get_bound_knowledge_base_ids(task)
end
end
rect rgb(255,250,240)
Note over ContextSvc,TaskKB: best-effort sync each KB context to task
loop for each KB id
ContextSvc->>TaskKB: sync_subtask_kb_to_task(db, task, kb_id, user_id, user_name)
TaskKB->>DB: fetch KnowledgeBase, Task, existing refs
TaskKB->>DB: check access (can_access_knowledge_base)
alt valid & not duplicate & under limit
TaskKB->>DB: append KnowledgeBaseTaskRef, persist task
TaskKB-->>ContextSvc: True
else
TaskKB-->>ContextSvc: False (logged, no raise)
end
end
end
ContextSvc-->>ChatNS: return linked context ids
ChatNS-->>Client: ack
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
backend/app/services/task_knowledge_base_service.py (1)
461-469: Consider using.is_(True)for SQLAlchemy boolean comparison.For consistency with other parts of the codebase (e.g.,
_get_bound_knowledge_base_idsincontexts.pyusesTaskResource.is_active.is_(True)), consider using the.is_()method.🔎 Proposed fix
kb = ( db.query(Kind) .filter( Kind.id == knowledge_id, Kind.kind == "KnowledgeBase", - Kind.is_active == True, + Kind.is_active.is_(True), ) .first() )backend/tests/services/test_task_knowledge_base_sync.py (2)
116-147: Consider removing or prefixing unused fixture arguments.Static analysis flags
mock_useras unused in this test. The fixture isn't needed sincemock_query.first.side_effectreturns the mocked objects directly without needing the fixture.🔎 Proposed fix
def test_sync_kb_to_task_already_bound( - self, service, mock_db, mock_knowledge_base, mock_user + self, service, mock_db, mock_knowledge_base ):Similar changes apply to
test_sync_kb_to_task_limit_reached(line 149) andtest_sync_kb_to_task_no_access(line 181).
272-284: Prefix unused unpacked variables with underscore.The
toolsandpromptvariables are unpacked but not used. Prefix them with_to satisfy linting.🔎 Proposed fix
- tools, prompt = _prepare_kb_tools_from_contexts( + _tools, _prompt = _prepare_kb_tools_from_contexts( kb_contexts=[kb_context], user_id=1, db=mock_db, base_system_prompt="Base prompt", task_id=100, user_subtask_id=1, )Apply similar changes at lines 303-310 and 333-340.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/app/api/ws/chat_namespace.pybackend/app/services/chat/preprocessing/contexts.pybackend/app/services/task_knowledge_base_service.pybackend/tests/services/test_task_knowledge_base_sync.py
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{py,ts,tsx,js,jsx}: All code comments MUST be written in English
File size MUST NOT exceed 1000 lines - split into multiple sub-modules if exceeded
Function length SHOULD NOT exceed 50 lines (preferred)
Files:
backend/app/services/task_knowledge_base_service.pybackend/tests/services/test_task_knowledge_base_sync.pybackend/app/api/ws/chat_namespace.pybackend/app/services/chat/preprocessing/contexts.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code MUST follow PEP 8, use Black formatter with line length 88, and isort for imports
Python code MUST include type hints
Python functions and classes MUST have descriptive names and docstrings for public functions/classes
Python MUST extract magic numbers to named constants
Files:
backend/app/services/task_knowledge_base_service.pybackend/tests/services/test_task_knowledge_base_sync.pybackend/app/api/ws/chat_namespace.pybackend/app/services/chat/preprocessing/contexts.py
backend/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
backend/**/*.py: Python backend module imports MUST useuv runprefix when executing commands
Task and Workspace resources MUST use TaskResource model fromapp.models.task, not the Kind model
Ghost, Model, Shell, Bot, Team, and Skill CRDs MUST use Kind model fromapp.models.kind
Files:
backend/app/services/task_knowledge_base_service.pybackend/tests/services/test_task_knowledge_base_sync.pybackend/app/api/ws/chat_namespace.pybackend/app/services/chat/preprocessing/contexts.py
backend/app/api/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
API routes MUST use CRD names (Team, Bot) in path names and database models
Files:
backend/app/api/ws/chat_namespace.py
🧬 Code graph analysis (4)
backend/app/services/task_knowledge_base_service.py (3)
backend/app/models/subtask_context.py (1)
knowledge_id(165-169)backend/app/models/kind.py (1)
Kind(25-42)backend/app/schemas/kind.py (1)
KnowledgeBaseTaskRef(415-421)
backend/tests/services/test_task_knowledge_base_sync.py (2)
backend/app/services/task_knowledge_base_service.py (1)
sync_subtask_kb_to_task(431-564)backend/app/services/chat/preprocessing/contexts.py (2)
_prepare_kb_tools_from_contexts(569-685)_get_bound_knowledge_base_ids(688-791)
backend/app/api/ws/chat_namespace.py (1)
frontend/e2e/fixtures/data-builders.ts (1)
task(212-219)
backend/app/services/chat/preprocessing/contexts.py (2)
backend/app/models/subtask_context.py (2)
SubtaskContext(46-193)knowledge_id(165-169)backend/app/services/task_knowledge_base_service.py (1)
sync_subtask_kb_to_task(431-564)
🪛 Ruff (0.14.10)
backend/app/services/task_knowledge_base_service.py
466-466: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
556-556: Consider moving this statement to an else block
(TRY300)
558-558: Do not catch blind exception: Exception
(BLE001)
backend/tests/services/test_task_knowledge_base_sync.py
117-117: Unused method argument: mock_user
(ARG002)
149-149: Unused method argument: mock_user
(ARG002)
181-181: Unused method argument: mock_task
(ARG002)
272-272: Unpacked variable tools is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
272-272: Unpacked variable prompt is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
303-303: Unpacked variable tools is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
303-303: Unpacked variable prompt is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
backend/app/services/chat/preprocessing/contexts.py
317-317: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: E2E Tests (Shard 1/3)
- GitHub Check: E2E Tests (Shard 3/3)
- GitHub Check: E2E Tests (Shard 2/3)
- GitHub Check: Test Frontend
- GitHub Check: Test wegent CLI Integration
🔇 Additional comments (8)
backend/app/api/ws/chat_namespace.py (1)
594-603: LGTM!The addition of
task_id=task.idcorrectly enables the new KB syncing functionality. Thetaskvariable is properly initialized in both code paths (direct chat at line 496 and non-direct chat at lines 538-546) before this point is reached.backend/app/services/task_knowledge_base_service.py (2)
549-550: Potential transaction conflict: commit inside a shared session.The
db.commit()here may interfere with the caller's transaction. If_sync_kb_contexts_to_taskincontexts.pyis called after_batch_update_and_insert_contextscommits, this should be fine. However, if the caller has uncommitted changes beyond what's been committed, this commit could unexpectedly flush them or the rollback on line 563 could undo the caller's work.Looking at the caller (
link_contexts_to_subtask), the commit at line 406 incontexts.pyhappens before_sync_kb_contexts_to_taskis called, so this should be safe. However, consider documenting this expectation or verifying the session state before committing.
431-457: Well-structured silent sync method with appropriate resilience.The method correctly implements the documented behavior:
- Silent skip on missing KB, no access, or limits reached
- Deduplication by name + namespace
- Proper user attribution with
boundByandboundAt- Exception handling with rollback to prevent partial state
The docstring clearly documents the differences from
bind_knowledge_base().backend/tests/services/test_task_knowledge_base_sync.py (1)
27-40: Comprehensive test coverage for the new sync functionality.The test suite covers all critical paths:
- Successful sync with proper KB binding and metadata
- Deduplication (already bound KB skipped)
- Limit enforcement (10 KB limit)
- Access control (no access → skip)
- Edge cases (KB not found, task not found)
- Priority logic (subtask KB > task-level KB)
- Fallback behavior (task-level KB used when no subtask KB)
backend/app/services/chat/preprocessing/contexts.py (4)
220-227: LGTM! Backward-compatible signature change.The new
task_idparameter is optional with a default ofNone, ensuring backward compatibility with existing callers. The docstring is properly updated to document the new parameter and behavior.
280-323: Well-designed best-effort sync helper.The function correctly:
- Extracts
knowledge_idfromtype_datasafely- Handles exceptions gracefully without blocking the main flow
- Logs success and failure cases appropriately
- Uses a local import to avoid potential circular import issues
The broad exception catch at line 317 is intentional for resilience, as this sync operation should never fail the main context linking flow.
601-624: Priority logic correctly implements subtask > task-level KB precedence.The implementation correctly prioritizes:
- Subtask-level KBs (user's explicit selection for this message)
- Task-level bound KBs (fallback when no subtask selection)
The logging clearly indicates which priority level is being used, which will help with debugging.
688-704: Docstring correctly updated to reflect removal of group chat restriction.The updated docstring accurately describes that the function now works for both group chat and non-group chat tasks, aligning with the PR objective to enable KB binding at the task level regardless of chat type.
…d objects (#847) Performance optimization for sync_subtask_kb_to_task method: - Changed task_id parameter to pre-queried task (TaskResource) object - Added user_name parameter to avoid internal user query - Updated link_contexts_to_subtask to accept task and user_name parameters - Modified _sync_kb_contexts_to_task to use the new parameters - Updated chat_namespace.py caller to pass pre-queried task and user_name - Removed obsolete test for task not found scenario - Updated existing tests to use new method signature This reduces database queries when syncing multiple knowledge bases to a task, avoiding redundant task and user lookups.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/services/chat/preprocessing/contexts.py (1)
581-641: Filter out invalidknowledge_idvalues when building subtask_kb_ids
SubtaskContext.knowledge_iddefaults to0when missing, and other callers (e.g.,extract_knowledge_base_ids,get_knowledge_base_ids_from_subtask) treat falsy values as “no KB”. Here you use:subtask_kb_ids = [c.knowledge_id for c in kb_contexts if c.knowledge_id is not None]This will include
0insubtask_kb_ids, which would propagate a bogus KB ID intoknowledge_base_idsand downstream tool construction.To keep semantics consistent and avoid invalid KB IDs, restrict to positive IDs:
Proposed fix for subtask_kb_ids filtering
- # Priority 1: Subtask-level knowledge bases (user-selected for this message) - subtask_kb_ids = [c.knowledge_id for c in kb_contexts if c.knowledge_id is not None] + # Priority 1: Subtask-level knowledge bases (user-selected for this message) + # Filter out falsy/invalid IDs (0 means "no KB" in SubtaskContext.knowledge_id) + subtask_kb_ids = [c.knowledge_id for c in kb_contexts if c.knowledge_id]This matches the rest of the codebase’s handling of
knowledge_idand avoids constructing tools with0as an ID.
🤖 Fix all issues with AI Agents
In @backend/tests/services/test_task_knowledge_base_sync.py:
- Around line 14-24: Tests import datetime and define a mock_user fixture that
isn't used; update the tests to remove unused imports and fixture usages: delete
the unused datetime import and either remove the mock_user fixture entirely and
drop mock_user from the signatures of test_sync_kb_to_task_success,
test_sync_kb_to_task_already_bound, test_sync_kb_to_task_limit_reached (and any
other tests referencing it), or modify those tests to reference the fixture's
properties (e.g., mock_user.id and mock_user.user_name) instead of hard-coded
values; look for the mock_user fixture definition and the test functions named
test_sync_kb_to_task_success, test_sync_kb_to_task_already_bound,
test_sync_kb_to_task_limit_reached to apply the change.
- Around line 234-299: The tests unpack the return of
_prepare_kb_tools_from_contexts into tools, prompt but never use them,
triggering Ruff RUF059; rename those locals to _tools and _prompt (or _ , _) in
both test_subtask_kb_takes_priority and
test_fallback_to_task_kb_when_no_subtask_kb so the calls remain for side effects
while satisfying the linter.
🧹 Nitpick comments (1)
backend/app/services/task_knowledge_base_service.py (1)
431-555: sync_subtask_kb_to_task logic looks solid; consider minor style tweaksThe overall behavior matches the PR intent: best‑effort append‑mode sync with deduplication, limit enforcement, access checks, and non‑fatal failures. A couple of small improvements to consider:
- SQLAlchemy boolean comparison style
You currently filter with:
Kind.is_active == TrueTo align with SQLAlchemy best practices and avoid E712‑style warnings, consider:
- .filter( - Kind.id == knowledge_id, - Kind.kind == "KnowledgeBase", - Kind.is_active == True, - ) + .filter( + Kind.id == knowledge_id, + Kind.kind == "KnowledgeBase", + Kind.is_active.is_(True), + )(Other occurrences elsewhere in the file can be updated separately for consistency.)
- Broad exception handler is intentional but noisy
Catching
Exceptionand returningFalseis reasonable here given the requirement that KB sync must never block chat flows. Since you already roll back and log atwarninglevel, the behavior is clear; no change strictly required, but if noisy in production you might later narrow this to persistence‑related exceptions.Functionally, though, this method is in good shape.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/app/api/ws/chat_namespace.pybackend/app/services/chat/preprocessing/contexts.pybackend/app/services/task_knowledge_base_service.pybackend/tests/services/test_task_knowledge_base_sync.py
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/api/ws/chat_namespace.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{py,ts,tsx,js,jsx}: All code comments MUST be written in English
File size MUST NOT exceed 1000 lines - split into multiple sub-modules if exceeded
Function length SHOULD NOT exceed 50 lines (preferred)
Files:
backend/tests/services/test_task_knowledge_base_sync.pybackend/app/services/task_knowledge_base_service.pybackend/app/services/chat/preprocessing/contexts.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code MUST follow PEP 8, use Black formatter with line length 88, and isort for imports
Python code MUST include type hints
Python functions and classes MUST have descriptive names and docstrings for public functions/classes
Python MUST extract magic numbers to named constants
Files:
backend/tests/services/test_task_knowledge_base_sync.pybackend/app/services/task_knowledge_base_service.pybackend/app/services/chat/preprocessing/contexts.py
backend/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
backend/**/*.py: Python backend module imports MUST useuv runprefix when executing commands
Task and Workspace resources MUST use TaskResource model fromapp.models.task, not the Kind model
Ghost, Model, Shell, Bot, Team, and Skill CRDs MUST use Kind model fromapp.models.kind
Files:
backend/tests/services/test_task_knowledge_base_sync.pybackend/app/services/task_knowledge_base_service.pybackend/app/services/chat/preprocessing/contexts.py
🧬 Code graph analysis (2)
backend/app/services/task_knowledge_base_service.py (4)
backend/app/models/task.py (1)
TaskResource(26-77)backend/app/models/subtask_context.py (1)
knowledge_id(165-169)backend/app/models/kind.py (1)
Kind(25-42)backend/app/schemas/kind.py (1)
KnowledgeBaseTaskRef(415-421)
backend/app/services/chat/preprocessing/contexts.py (3)
backend/app/models/task.py (1)
TaskResource(26-77)backend/app/models/subtask_context.py (2)
SubtaskContext(46-193)knowledge_id(165-169)backend/app/services/task_knowledge_base_service.py (1)
sync_subtask_kb_to_task(431-554)
🪛 Ruff (0.14.10)
backend/tests/services/test_task_knowledge_base_sync.py
78-78: Unused method argument: mock_user
(ARG002)
118-118: Unused method argument: mock_user
(ARG002)
151-151: Unused method argument: mock_user
(ARG002)
255-255: Unpacked variable tools is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
255-255: Unpacked variable prompt is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
286-286: Unpacked variable tools is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
286-286: Unpacked variable prompt is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
backend/app/services/task_knowledge_base_service.py
468-468: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
546-546: Consider moving this statement to an else block
(TRY300)
548-548: Do not catch blind exception: Exception
(BLE001)
backend/app/services/chat/preprocessing/contexts.py
226-226: Undefined name TaskResource
(F821)
290-290: Undefined name TaskResource
(F821)
329-329: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: E2E Tests (Shard 1/3)
- GitHub Check: E2E Tests (Shard 2/3)
- GitHub Check: E2E Tests (Shard 3/3)
- GitHub Check: Test wegent CLI Integration
🔇 Additional comments (4)
backend/tests/services/test_task_knowledge_base_sync.py (1)
225-408: Good coverage of new KB priority and task‑level binding behaviorThe tests under
TestKBPriorityLogicandTestGetBoundKnowledgeBaseIdsnicely pin down:
- Subtask KBs taking precedence over task‑level refs.
- Fallback to task‑level KBs when the subtask has none.
- Behavior when both levels are empty.
- Task‑level KB ID resolution for both group and non‑group chats, including empty refs and missing tasks.
This aligns well with the intended priority rules and task‑level reuse semantics.
backend/app/services/chat/preprocessing/contexts.py (3)
220-285: KB sync wiring in link_contexts_to_subtask is correct and safely gatedThe new
taskanduser_nameparameters and the_sync_kb_contexts_to_taskcall are guarded with:if task and kb_contexts_to_create and user_name: _sync_kb_contexts_to_task(...)This keeps existing callers working (both args are optional) and ensures KB sync runs only when all required context is available. The transactional split—first committing context creation, then best‑effort task‑level sync—is consistent with the “non‑blocking sync” requirement.
No functional issues here.
287-335: Best‑effort sync helper is aligned with requirements; broad catch is acceptable
_sync_kb_contexts_to_taskcorrectly:
- Iterates only KB contexts with a
knowledge_id.- Delegates to
task_knowledge_base_service.sync_subtask_kb_to_taskfor each KB.- Logs success per KB and, on any exception, logs a warning without propagating (so chat is never blocked).
The broad
except Exceptionis justified here given the design goal of making task‑level sync non‑critical to the main flow, and the inline comment documents this clearly.No changes needed from a functional perspective.
698-801: _get_bound_knowledge_base_ids implementation matches new task‑level semanticsThe updated
_get_bound_knowledge_base_ids:
- Uses
TaskResourceand no longer filters byis_group_chat, as intended.- Reads
knowledgeBaseRefsfrom task JSON and resolves them to KB IDs by matchingspec.nameper namespace.- Logs useful diagnostics and guards all errors to avoid impacting chat.
This is consistent with the new tests that expect KB IDs for both group and non‑group chat tasks and aligns with the max‑10 KB binding logic enforced elsewhere.
Looks good.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/tests/services/test_task_knowledge_base_sync.py (1)
25-208: Comprehensive test coverage with proper mocking patterns.The test class covers all critical scenarios: successful sync, deduplication, limit enforcement, access control, and not-found handling. Excellent use of
Mock(spec=...)to catch attribute errors early, and proper verification of both return values and side effects (commits).Optional: Extract repeated mock_query setup
The pattern of setting up
mock_query(lines 71-76, 119-122, 150-153, 172-175, 192-195) could be extracted to a fixture if you prefer DRY, but the current explicit approach is clear and maintainable.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/tests/services/test_task_knowledge_base_sync.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{py,ts,tsx,js,jsx}: All code comments MUST be written in English
File size MUST NOT exceed 1000 lines - split into multiple sub-modules if exceeded
Function length SHOULD NOT exceed 50 lines (preferred)
Files:
backend/tests/services/test_task_knowledge_base_sync.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code MUST follow PEP 8, use Black formatter with line length 88, and isort for imports
Python code MUST include type hints
Python functions and classes MUST have descriptive names and docstrings for public functions/classes
Python MUST extract magic numbers to named constants
Files:
backend/tests/services/test_task_knowledge_base_sync.py
backend/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
backend/**/*.py: Python backend module imports MUST useuv runprefix when executing commands
Task and Workspace resources MUST use TaskResource model fromapp.models.task, not the Kind model
Ghost, Model, Shell, Bot, Team, and Skill CRDs MUST use Kind model fromapp.models.kind
Files:
backend/tests/services/test_task_knowledge_base_sync.py
🧠 Learnings (1)
📚 Learning: 2025-12-31T03:47:12.173Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-31T03:47:12.173Z
Learning: Applies to **/*.test.py : Python tests MUST mock external services (Anthropic, OpenAI, Docker, APIs)
Applied to files:
backend/tests/services/test_task_knowledge_base_sync.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test wegent CLI Integration
- GitHub Check: Test Frontend
- GitHub Check: E2E Tests (Shard 3/3)
- GitHub Check: E2E Tests (Shard 1/3)
- GitHub Check: E2E Tests (Shard 2/3)
🔇 Additional comments (3)
backend/tests/services/test_task_knowledge_base_sync.py (3)
1-23: Excellent module structure and organization.The imports are properly organized according to PEP 8/isort standards (stdlib → third-party → local), and the module docstring clearly explains the test coverage. Past review comments regarding unused imports have been addressed.
210-313: Priority logic tests are well-implemented and correctly address past feedback.The tests properly validate that subtask-level KBs take precedence over task-level KBs, with appropriate fallback behavior. The use of underscore-prefixed variables (
_tools, _prompt) on lines 240 and 271 correctly signals unused return values, while line 301 properly uses unprefixed names since those values are asserted. Good defensive programming withset()comparison on line 283.
315-393: Excellent validation of the behavioral change for non-group chat tasks.The test
test_get_bound_kb_ids_for_non_group_chat(line 324) explicitly validates that the previous restriction limiting KB bindings to group chats has been removed, which aligns with the PR objectives. Proper edge case coverage for empty refs and missing tasks ensures graceful degradation.
Implement automatic synchronization of subtask-level knowledge base selections to task-level knowledgeBaseRefs, enabling KB reuse across all subtasks within a task.
Changes:
Key features:
Summary by CodeRabbit
New Features
Behavior / Reliability
Tests
✏️ Tip: You can customize this high-level summary in your review settings.