fix: Resolve #1529 - fix: Resolve #1528 - fix: enforce ALLOWED_UPDATE_FIELDS in update_memory (#770)#1530
Conversation
Signed-off-by: washim0988-art <islowashin@gmail.com>
|
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)
📝 WalkthroughWalkthroughAdds an allowlist for memory record updates, initializes optional fields to ChangesMemory update flow
Estimated code review effort: 2 (Simple) | ~10 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@fix_1529.py`:
- Around line 36-44: Update update_memory to accept an existing
MemoryRecordRepository argument instead of creating one internally, and remove
the unconditional add_record call so missing-record validation remains
reachable. Remove the ValueError catch-and-print behavior and let
repository.update_record exceptions propagate to the caller; update the
invocation to pass the repository dependency.
- Around line 11-17: Update the update method to use a module-level or
class-level ALLOWED_UPDATE_FIELDS constant instead of defining
allowed_update_fields locally, and preserve validation against that constant.
Change the ValueError text for unsupported keys to the downstream-required
message “Unknown update field”.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Refactor MemoryRecord to use constants for allowed fields and improve error handling. Update update_memory function to accept repository as an argument.
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 `@fix_1529.py`:
- Around line 15-21: Update update to validate all keys in updates against
ALLOWED_UPDATE_FIELDS before mutating any attributes, preserving atomicity when
invalid keys are present. Collect the invalid keys and raise ValueError
containing the complete invalid-key list; only apply setattr after validation
succeeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Validate update keys before applying changes to ensure atomic updates. Improved error handling for unknown fields.
|
I believe this is a duplicate |
Resolves #1529
Built autonomously by Cloud Agent.
Bounty payout address: Gq46qirFLJY3qptAWkAmAeDfGVAE4MYYGTcRmpKjsyR
Summary by CodeRabbit