Feat/kaizen policies - #68
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a generic MCP tool Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant MCP as MCP_Server
participant Search as EntityStore/Search
participant FS as Filesystem
Client->>MCP: get_entities(task, entity_type)
MCP->>Search: search_entities(filters: {type: entity_type}, task)
Search-->>MCP: list of Entity objects
MCP->>FS: read/ensure_namespace metadata
FS-->>MCP: namespace metadata (num_entities, etc.)
MCP-->>Client: formatted response ("# {EntityType}s for: {task}" + items)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@kaizen/backend/filesystem.py`:
- Around line 222-223: The delete path leaves persisted
FilesystemNamespace.num_entities stale because we only update num_entities when
adding; in delete_entity_by_id update the in-memory namespace data's
num_entities (set data.num_entities = len(data.entities) or equivalent) before
calling self._save_namespace_data so saved metadata stays in sync with the
actual entities; modify delete_entity_by_id to update num_entities and then call
_save_namespace_data.
In `@tests/unit/test_policy_schema.py`:
- Around line 1-8: Add the pytest "unit" marker to the test module so it runs
under -m unit: either set a module-level marker by adding pytestmark =
pytest.mark.unit at the top of tests/unit/test_policy_schema.py (above imports
or right after them) or decorate each test function with `@pytest.mark.unit`;
ensure you import pytest if not already present.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
AGENTS.mdREADME.mddocs/POLICIES.mdkaizen/backend/filesystem.pykaizen/frontend/mcp/mcp_server.pykaizen/schema/policy.pytests/unit/test_policy_schema.py
- Added Policy schema mirroring CUGA models - Enhanced MCP server with generic get_entities tool - Restored get_guidelines as a backward-compatible alias - Fixed num_entities bug in Filesystem backend - Added dedicated policy documentation and unit tests
3039262 to
ec4c0a9
Compare
This commit fixes the stale num_entities when deleting a filesystem entity, removes the missing ensure_namespace function call from the mcp_server after the upstream rebase, and adds a module-level pytest unit mark to the policy schema tests.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
kaizen/frontend/mcp/mcp_server.py (1)
78-78:capitalize()produces malformed headers for underscore-separated entity types.
"intent_guard".capitalize()→"Intent_guard", giving headers like# Intent_guards for: …. Consider replacing underscores before capitalising.✨ Suggested improvement
- header = f"# {entity_type.capitalize()}s for: {task}" + header = f"# {entity_type.replace('_', ' ').title()}s for: {task}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kaizen/frontend/mcp/mcp_server.py` at line 78, The header generation uses entity_type.capitalize() which leaves underscores intact (e.g., "Intent_guard"); update the header creation (the header variable assignment) to replace underscores and properly title-case the entity type before pluralizing—e.g., transform entity_type with entity_type.replace("_", " ").title() (or split and capitalize each word) and then use that result in f"# {…}s for: {task}" so headers read "Intent Guards for: …".kaizen/schema/policy.py (1)
26-26:thresholdhas no range constraint — invalid values (e.g.2.5,-0.1) are silently accepted.🛡️ Suggested validation
- threshold: float = 0.7 # for natural_language triggers + threshold: float = Field(default=0.7, ge=0.0, le=1.0) # for natural_language triggers🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kaizen/schema/policy.py` at line 26, The threshold field currently accepts any float; add validation to enforce 0.0 <= threshold <= 1.0 by adding a check where the containing class is initialized (e.g., implement a __post_init__ for dataclasses or a validate_threshold/validator for pydantic models) that raises ValueError if threshold is out of range, or replace the declaration with a pydantic Field(threshold: float = Field(0.7, ge=0.0, le=1.0)) to enforce the bounds automatically; reference the threshold symbol and the class that declares it when adding this validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/POLICIES.md`:
- Around line 17-19: The TriggerType.ALWAYS option is missing from the trigger
list in the docs; update the "Trigger Types" section to include `always` (e.g.,
describe that `always` triggers apply to every input/response such as global
output formatters) so the docs match the policy.py schema and references to
TriggerType.ALWAYS.
In `@kaizen/frontend/mcp/mcp_server.py`:
- Around line 66-84: The call to the undefined function ensure_namespace()
inside get_entities_logic causes a NameError and is redundant because
get_client() already initializes the namespace; remove the ensure_namespace()
call from get_entities_logic (and any other occurrences in this module such as
in get_entities or get_guidelines if present) so the function relies on
get_client()'s internal _namespace_initialized logic and no longer references
the undefined symbol.
In `@kaizen/schema/policy.py`:
- Around line 1-3: Replace deprecated typing generics in the imports and model
annotations: remove List, Dict, Optional from the typing import and use built-in
generics (list, dict) and PEP 604 union syntax (T | None) for optional fields
used in your Pydantic models; update any field annotations that referenced
List[...] to list[...], Dict[...] to dict[..., ...], and Optional[T] to T |
None, keeping Enum, BaseModel, Field, and Any as needed (e.g., adjust model
classes that reference these types in kaizen/schema/policy.py such as any
BaseModel subclasses and their field definitions).
---
Nitpick comments:
In `@kaizen/frontend/mcp/mcp_server.py`:
- Line 78: The header generation uses entity_type.capitalize() which leaves
underscores intact (e.g., "Intent_guard"); update the header creation (the
header variable assignment) to replace underscores and properly title-case the
entity type before pluralizing—e.g., transform entity_type with
entity_type.replace("_", " ").title() (or split and capitalize each word) and
then use that result in f"# {…}s for: {task}" so headers read "Intent Guards
for: …".
In `@kaizen/schema/policy.py`:
- Line 26: The threshold field currently accepts any float; add validation to
enforce 0.0 <= threshold <= 1.0 by adding a check where the containing class is
initialized (e.g., implement a __post_init__ for dataclasses or a
validate_threshold/validator for pydantic models) that raises ValueError if
threshold is out of range, or replace the declaration with a pydantic
Field(threshold: float = Field(0.7, ge=0.0, le=1.0)) to enforce the bounds
automatically; reference the threshold symbol and the class that declares it
when adding this validation.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
AGENTS.mdREADME.mddocs/POLICIES.mdkaizen/backend/filesystem.pykaizen/frontend/mcp/mcp_server.pykaizen/schema/policy.pytests/unit/test_policy_schema.py
🚧 Files skipped from review as they are similar to previous changes (2)
- kaizen/backend/filesystem.py
- tests/unit/test_policy_schema.py
Modernized typing generics in policy schema using built-in list/dict and PEP 604 union syntax. Updated documentation to include the missing "always" trigger type.
|
ready to be reviewed |
|
@visahak just wanna make sure i understand it correctly, this change is to support create policy manually and store it in kaizen but extract policies from trajectories is not supported yet? |
Yes |
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests