Skip to content

feat: support filtered session buffer flush - #329

Merged
xxynet merged 1 commit into
dev-v3from
feat/session-buffer-flush-filter
Sep 25, 2026
Merged

xxynet merged 1 commit into
dev-v3from
feat/session-buffer-flush-filter

Conversation

@xxynet

@xxynet xxynet commented Sep 25, 2026 •

Copy link
Copy Markdown
Owner

⚠️ Base branch must be dev. PRs targeting main will be rejected.

Summary

Add predicate-based filtering to SessionBuffer.flush so callers can flush selected messages while retaining unmatched messages in the buffer.

Type of change

  • feat: New feature
  • fix: Bug fix
  • refactor: Code refactor (no functional change)
  • docs: Documentation update
  • chore: Build, CI, dependencies, or other maintenance
  • style: Code style / formatting (no logic change)
  • test: Adding or updating tests

Changes

  • Add an optional filter_fn predicate to SessionBuffer.flush.
  • Give filter_fn precedence over count; use count only when no filter is provided.
  • Keep unmatched messages in their original buffer order.
  • Add tests for filtering, precedence, count-only behavior, default behavior, and no matches.

Screenshots / Logs

  • python -m pytest tests/test_message_manager.py tests/test_chat_plugin.py tests/test_message_utils.py -v
  • Result: 50 passed.

Checklist

  • I have tested these changes locally
  • I have updated documentation if needed
  • New dependencies have been added to requirements.txt (if applicable)
  • If this PR introduces new features, they have been discussed with the owner or maintainer
  • This PR does not introduce breaking changes

Summary by CodeRabbit

  • New Features
    • Buffered messages can now be retrieved selectively using a matching condition. Matching messages are returned in their original order, while messages that don’t match remain buffered.
    • When a matching condition is provided, it takes precedence over the requested count. Without a condition, count-based retrieval remains available, including retrieving a specified number of messages or flushing the full buffer by default.

@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 172e42d6-7806-4441-8a8b-2535e50f9b8a

📥 Commits

Reviewing files that changed from the base of the PR and between cbf4d2f and cbf4d2f.

📒 Files selected for processing (1)
  • core/message_manager.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

SessionBuffer.flush now accepts an optional predicate. With a predicate, it returns and removes matching messages while retaining nonmatching messages. Without a predicate, it keeps the existing count-based behavior. Tests cover both paths and empty matches.

Changes

SessionBuffer flush behavior

Layer / File(s) Summary
Predicate-based flush and validation
core/message_manager.py, tests/test_message_manager.py
SessionBuffer.flush accepts an optional filter_fn. It removes and returns matching messages in buffer order, regardless of count. Without a predicate, existing count-based behavior remains. Tests check filtered, count-limited, default, and no-match cases.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Merge Risk: ⚪ Minimal · up to cbf4d

Filtered flush preserves the intended message selection and retention behavior; no merge-blocking issue was identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to cbf4d

The existing message-delivery path is unchanged, and no new attacker-controlled path was identified. A predicate that changes the buffer while it is being evaluated could nevertheless cause incorrect message removal; callers outside this repository and their usage are unknown.

Retained concerns

  • Low · reliability · inferred: A predicate that mutates or recursively flushes the buffer can invalidate the indices used for removal, potentially returning or removing the wrong messages. No repository-local production caller supplies a predicate, so current production exposure is not established.
Security review details

Security Blast Radius

  • inferred — The observed production effect remains within session-keyed buffering and its existing batch-publication path; exposure through consumers outside this repository is unknown.

Trust Boundaries and Controls

  • observed — Production callers hold the per-session lock across addition or removal. The new predicate is caller-supplied code, but no repository-local production caller passes one.

Resilience and Maintainability Implications

  • inferred — Caller-held locking serializes the identified production operations, but it does not protect index-based selection against a predicate that changes the same buffer synchronously.

Hardening Proposals

  • proposed — Define whether predicates must be pure and non-reentrant, or make filtered selection resilient to buffer mutation before this API is used for message delivery.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding filtered flush support to the session buffer.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

A rabbit checks the message row,
Then picks the matches in their flow.
The others stay within the queue,
Count-only flushes still behave as due.
No match? The buffer keeps its place.
The rabbit hops off, pleased with grace.

Comment @coderabbitai help to get the list of available commands.

@xxynet
xxynet changed the base branch from dev to dev-v3 September 25, 2026 04:24
@xxynet
xxynet merged commit 2983474 into dev-v3 Sep 25, 2026
4 checks passed
@xxynet
xxynet deleted the feat/session-buffer-flush-filter branch September 25, 2026 04:31
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.

1 participant