Skip to content

Conversation

@abrookins
Copy link
Collaborator

This PR fixes discrepancies between the memory lifecycle documentation and the actual implementation.

Changes

  • Removed client-controlled forgetting APIs that don't exist
  • Documented background task system using Docket
  • Clarified server autonomy over memory lifecycle management
  • Updated configuration examples to match actual implementation
  • Added best practices for robust client design

Closes #65

Generated with Claude Code

… implementation

- Remove client-controlled forgetting APIs that don't exist
- Document background task system using Docket
- Clarify server autonomy over memory lifecycle management
- Update configuration examples to match actual implementation
- Add best practices for robust client design

Co-authored-by: Andrew Brookins <abrookins@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 5, 2025 16:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes critical discrepancies between the documentation and the actual implementation by rewriting the memory lifecycle documentation to accurately reflect the server-controlled memory management system.

  • Removed non-existent client-controlled forgetting APIs and replaced with server-side background process documentation
  • Added comprehensive coverage of the Docket-based background task system for memory lifecycle operations
  • Updated configuration examples and client usage patterns to match the actual implementation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

for i in range(0, len(memory_ids), batch_size):
batch_ids = memory_ids[i:i + batch_size]
await client.delete_memories(batch_ids)
await client.delete_long_term_memories(batch_ids) # Correct method name
Copy link

Copilot AI Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The comment 'Correct method name' should be removed as it's not providing value to readers and appears to be a note from the documentation rewrite process.

Copilot uses AI. Check for mistakes.
memory_ids = [mem.id for mem in topic_memories.memories]
if memory_ids:
await client.delete_memories(memory_ids)
await client.delete_long_term_memories(memory_ids) # Correct method name
Copy link

Copilot AI Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The comment 'Correct method name' should be removed as it's not providing value to readers and appears to be a note from the documentation rewrite process.

Copilot uses AI. Check for mistakes.
else:
return get_default_preference(preference_key)
# Bad: Assuming specific memories will always exist
Copy link

Copilot AI Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commented code example references get_memory_by_id() method which may not exist in the actual API. Consider using a method that's documented elsewhere in the codebase or clarify that this is a hypothetical example.

Suggested change
# Bad: Assuming specific memories will always exist
# Bad: Assuming specific memories will always exist
# Hypothetical: get_memory_by_id() does not exist in the real API

Copilot uses AI. Check for mistakes.
@abrookins
Copy link
Collaborator Author

@claude Tighten the language in this PR: no need to reinforce that forgetting is “not controlled by the client.” Just talk about how it works.

@claude
Copy link
Contributor

claude bot commented Sep 5, 2025

Claude finished @abrookins's task —— View job


Tasks

  • Read current memory-lifecycle.md documentation
  • Remove unnecessary reinforcement about client control limitations
  • Clean up leftover editing comments ("Correct method name")
  • Focus language on how the system works positively
  • Commit and push changes

Changes Made

Tightened the language throughout the memory lifecycle documentation:

Key improvements:

  • Removed warning box (line 18) that over-emphasized client limitations
  • Eliminated negative language like "clients cannot" and "not controlled by the client"
  • Focused on positive descriptions of how the system works
  • Cleaned up leftover comments like "Correct method name" from lines 242, 264
  • Fixed code comment referencing non-existent API method (line 378)

Examples of changes:

  • "Memory forgetting is controlled entirely by the server, not the client" → "Memory forgetting operates through server-controlled background processes"
  • "While clients cannot control forgetting policies, they can:" → "Clients can perform direct memory management operations:"
  • "Important: Clients cannot directly trigger compaction" → "Compaction runs automatically through background tasks"

The documentation now focuses on how the system works rather than what it doesn't allow, making it more actionable and less defensive.

- Remove unnecessary reinforcement about client control limitations
- Focus on positive descriptions of how the system works
- Clean up leftover editing comments ('Correct method name')
- Address review feedback for cleaner, more direct language

Co-authored-by: Andrew Brookins <abrookins@users.noreply.github.com>
@abrookins
Copy link
Collaborator Author

abrookins commented Sep 5, 2025

@claude Fix the linting and make sure you use "pre-commit"

@claude
Copy link
Contributor

claude bot commented Sep 5, 2025

Claude finished @abrookins's task —— View job


Claude finished @abrookins's task —— View job


Tasks

  • Set up development environment and activate venv
  • Install dependencies and pre-commit hooks
  • Run linting tools to identify issues
  • Run pre-commit on all files
  • Fix any linting/formatting issues found
  • Commit and push the fixes

Changes Made

Successfully fixed linting issues and ensured pre-commit compliance:

Linting fixes applied:

  • Trailing whitespace removed from 16 lines in docs/memory-lifecycle.md
  • Code formatting applied with ruff across the codebase
  • All pre-commit hooks now passing

Pre-commit hooks verified:

  • ✅ ruff (linting)
  • ✅ ruff-format (code formatting)
  • ✅ trim trailing whitespace
  • ✅ fix end of files
  • ✅ check yaml
  • ✅ check for added large files
  • ✅ typos

The documentation is now properly formatted and all automated code quality checks are passing. The commit includes the fixes and has been pushed to the PR branch.

- Remove trailing whitespace from multiple lines
- Apply code formatting with ruff
- Ensure all pre-commit hooks pass

Co-authored-by: Andrew Brookins <abrookins@users.noreply.github.com>
@abrookins abrookins merged commit f91a701 into main Sep 5, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rewrite lifecycle docs

2 participants