Conversation
There was a problem hiding this comment.
Pull request overview
Implements a “dynamic injection” mechanism (replacing the prior attachment-based approach) to inject plan mode reminders into the prompt/history before each LLM step, with updated provider logic and tests to match.
Changes:
- Replace attachment concepts with
DynamicInjection/DynamicInjectionProviderand migrate plan mode reminder provider accordingly. - Update
KimiSoulto collect and inject dynamic injections each step, then normalize history for API input. - Update/rename tests and imports to cover the new dynamic injection flow and reminder detection.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/kimi_cli/soul/kimisoul.py |
Switches from attachment collection/injection to dynamic injection collection/injection within the step pipeline. |
src/kimi_cli/soul/dynamic_injection.py |
Introduces the core DynamicInjection abstraction + provider interface and history normalization behavior. |
src/kimi_cli/soul/dynamic_injections/plan_mode.py |
Migrates plan mode reminder logic to a DynamicInjectionProvider implementation. |
src/kimi_cli/soul/dynamic_injections/__init__.py |
Package init for dynamic injection providers. |
tests/core/test_plan_mode_reminder.py |
Updates reminder detection tests to import from dynamic injection plan mode module. |
tests/core/test_plan_mode_injection_provider.py |
Updates provider flow tests to use PlanModeInjectionProvider.get_injections(). |
tests/core/test_plan_mode.py |
Renames and updates manual plan mode toggle tests to validate pending injection behavior. |
tests/core/test_normalize_history.py |
Updates normalization tests to import from dynamic_injection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async def toggle_plan_mode(self) -> bool: | ||
| """Toggle plan mode on/off. Returns the new state. | ||
|
|
||
| Tools are not hidden/unhidden — instead, each tool checks plan mode | ||
| state at call time and rejects if blocked. | ||
| Periodic reminders are handled by the attachment system. | ||
| Periodic reminders are handled by the dynamic injection system. | ||
| """ |
There was a problem hiding this comment.
The PR description still contains placeholder text (Resolve #(issue_number)) and does not link an actual related issue. Please update the PR metadata to reference the real issue/decision thread (or remove the placeholder if none applies) so reviewers have the correct context for this behavior change in the core LLM step pipeline.
Summary
Attachment/AttachmentProvidertoDynamicInjection/DynamicInjectionProviderto better reflect the system's purpose as a dynamic prompt injection mechanismsoul/attachment.py→soul/dynamic_injection.py,soul/attachments/→soul/dynamic_injections/_collect_injections()so a failing provider doesn't block the entire LLM stepTest plan
test_normalize_history,test_plan_mode_injection_provider,test_plan_mode_reminder,test_plan_mode)grep -rconfirms zero remaining references to old namingChecklist
make gen-changelogto update the changelog.make gen-docsto update the user documentation.