Skip to content

fix: wire compact_semantic_duplicates setting to env var#182

Open
Piotr1215 wants to merge 3 commits intoredis:mainfrom
Piotr1215:fix/compact-semantic-duplicates-upstream
Open

fix: wire compact_semantic_duplicates setting to env var#182
Piotr1215 wants to merge 3 commits intoredis:mainfrom
Piotr1215:fix/compact-semantic-duplicates-upstream

Conversation

@Piotr1215
Copy link
Contributor

@Piotr1215 Piotr1215 commented Mar 5, 2026

Summary

  • Add compact_semantic_duplicates field to Settings in config.py (default True)
  • Change compact_long_term_memories() to read the default from settings.compact_semantic_duplicates instead of hardcoded True

The compact_semantic_duplicates parameter was hardcoded to True with no corresponding Settings field. Setting COMPACT_SEMANTIC_DUPLICATES=false as an env var had no effect because pydantic-settings never read it. The Docket perpetual scheduler always called the function with the hardcoded default.

Test plan

  • Verify COMPACT_SEMANTIC_DUPLICATES=false produces semantic_duplicates=False in worker logs
  • Verify default behavior unchanged (semantic compaction still runs when env var is unset)

Note

Low Risk
Low risk: small configuration plumbing change that only alters compaction behavior when COMPACT_SEMANTIC_DUPLICATES is set, plus non-functional notebook formatting tweaks.

Overview
Makes semantic compaction configurable via env var. Adds compact_semantic_duplicates to Settings (default True) and updates compact_long_term_memories() to default to settings.compact_semantic_duplicates when no explicit argument is passed.

Adds a regression test to ensure COMPACT_SEMANTIC_DUPLICATES=false is read by Settings, and includes minor formatting/cleanup edits in the interactive guide notebook.

Written by Cursor Bugbot for commit 94a5ede. This will update automatically on new commits. Configure here.

the compact_semantic_duplicates parameter in compact_long_term_memories()
was hardcoded to True with no corresponding Settings field. setting
COMPACT_SEMANTIC_DUPLICATES=false as an env var had no effect because
pydantic-settings never read it. add the field to Settings so the
docket perpetual scheduler respects the env var.
Copilot AI review requested due to automatic review settings March 5, 2026 10:13
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

Wires the long-term memory semantic deduplication toggle to a real Settings field so COMPACT_SEMANTIC_DUPLICATES=false can disable semantic compaction in scheduled/worker runs.

Changes:

  • Add compact_semantic_duplicates: bool = True to Settings in config.py.
  • Update compact_long_term_memories() to default compact_semantic_duplicates from settings.compact_semantic_duplicates instead of a hardcoded True.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
agent_memory_server/long_term_memory.py Reads the semantic-duplicate compaction default from settings so env-configured behavior can take effect.
agent_memory_server/config.py Adds the missing Settings field so COMPACT_SEMANTIC_DUPLICATES is actually parsed by pydantic-settings.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Piotr1215 added a commit to Piotr1215/homelab that referenced this pull request Mar 5, 2026
wires COMPACT_SEMANTIC_DUPLICATES env var to config so the worker
actually respects it. upstream PR: redis/agent-memory-server#182
the default was evaluated at function definition time, meaning
test-time patches and runtime config reloads had no effect.
use None sentinel with runtime resolution instead.

add regression test for COMPACT_SEMANTIC_DUPLICATES env var parsing.
Copy link
Contributor

@vishal-bala vishal-bala left a comment

Choose a reason for hiding this comment

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

This LGTM! You need to run make format on your code for it to pass the linters.

make format uses a different ruff version than the pre-commit hook
(v0.3.2). re-ran with pre-commit run --all-files to match CI.
@Piotr1215 Piotr1215 force-pushed the fix/compact-semantic-duplicates-upstream branch from 94c5e78 to 94a5ede Compare March 13, 2026 12:55
@Piotr1215
Copy link
Contributor Author

Piotr1215 commented Mar 15, 2026

hey @vishal-bala, ran make format linter passes now. the failing CI tests (39 of them) are all OPENAI_API_KEY 401s because GitHub doesn't expose repo secrets to fork PRs. same tests pass fine on main. Mind taking another look? thanks!

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.

3 participants