Skip to content

Disable summarization feature#226

Merged
rbs333 merged 3 commits intomainfrom
bug/raae-193
Mar 18, 2026
Merged

Disable summarization feature#226
rbs333 merged 3 commits intomainfrom
bug/raae-193

Conversation

@rbs333
Copy link
Contributor

@rbs333 rbs333 commented Mar 17, 2026

This pr addresses: #193

Description

This PR adds an explicit configuration flag to disable working-memory summarization and validates the SUMMARIZATION_THRESHOLD setting to prevent misconfiguration.

Problem

Previously, users who wanted to disable working-memory summarization had no clean way to do so. Setting SUMMARIZATION_THRESHOLD to extreme values (like 0 or 100.0) could
cause undefined behavior or subtle bugs. There was no fail-fast validation for invalid threshold values.

Solution

1. New config option: ENABLE_WORKING_MEMORY_SUMMARIZATION (default: true)
   • When set to false, working memory summarization is skipped entirely during PUT /v1/working-memory/{session_id} operations
   • Messages are preserved as-is without token-based trimming or summarization

2. Threshold validation: SUMMARIZATION_THRESHOLD now validates that values are in the range (0, 1]
   • Invalid values (≤0 or >1) fail at startup with a clear error message
   • Error message guides users to use ENABLE_WORKING_MEMORY_SUMMARIZATION=false instead

Changes

File Changes
agent_memory_server/config.py Added enable_working_memory_summarization setting and @field_validator for threshold validation
agent_memory_server/api.py Added early return in _summarize_working_memory() when summarization is disabled
tests/test_summarization.py Added 9 tests for threshold validation and enable flag
tests/test_api.py Added integration test verifying summarization is skipped when disabled

Usage

Disable summarization

ENABLE_WORKING_MEMORY_SUMMARIZATION=false

Enable summarization (default)

ENABLE_WORKING_MEMORY_SUMMARIZATION=true
SUMMARIZATION_THRESHOLD=0.7 # Must be in (0, 1]

Testing

• ✅ All unit tests pass (10 new tests added)
• ✅ Docker-based integration test validates end-to-end behavior
• ✅ Pre-commit checks pass

Breaking Changes

None. Default behavior is unchanged (enable_working_memory_summarization=true).

Related

Closes #193


Note

Medium Risk
Behavior of PUT /v1/working-memory/{session_id} can now change significantly based on config, and stricter summarization_threshold validation may cause startup failures for previously accepted (invalid) environment values.

Overview
Adds an explicit enable_working_memory_summarization setting (default True) and makes _summarize_working_memory in api.py return early when it’s disabled, preserving all messages and skipping summary/context trimming.

Tightens configuration by validating summarization_threshold to (0, 1] via Pydantic field constraints, and adds tests covering the new flag, threshold validation, and an API-level case ensuring summarization is skipped when disabled.

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

Copilot AI review requested due to automatic review settings March 17, 2026 15:56
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Review completed. The implementation is well-structured with comprehensive test coverage. Found one potential issue with test isolation that should be addressed.

@jit-ci
Copy link

jit-ci bot commented Mar 17, 2026

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

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

Adds an explicit configuration switch to disable working-memory summarization and enforces validation on the summarization threshold to prevent invalid configurations (addressing issue #193).

Changes:

  • Introduces enable_working_memory_summarization (env: ENABLE_WORKING_MEMORY_SUMMARIZATION, default true) to skip summarization during working-memory PUT operations.
  • Validates summarization_threshold to ensure it is within (0, 1], failing fast with a guidance message when invalid.
  • Adds unit + integration tests covering the new flag behavior and threshold validation.

Reviewed changes

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

File Description
agent_memory_server/config.py Adds enable_working_memory_summarization and validates summarization_threshold range at settings load time.
agent_memory_server/api.py Skips _summarize_working_memory() entirely when summarization is disabled.
tests/test_summarization.py Adds unit tests for threshold validation and enable/disable flag default/override behavior.
tests/test_api.py Adds an integration-style test ensuring PUT preserves messages and context when summarization is disabled.

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

@vishal-bala vishal-bala self-requested a review March 18, 2026 08:50
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.

LGTM! There's one bot comment (that I added a follow-up comment on) that I think would be good to address, and a couple of very minor ones from me.

Copy link
Collaborator

@abrookins abrookins left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for plugging that!

@rbs333 rbs333 merged commit fd73560 into main Mar 18, 2026
23 checks passed
@rbs333 rbs333 deleted the bug/raae-193 branch March 18, 2026 20:12
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.

Unable to actually disable summarization features

4 participants