Skip to content

fix: Fix historyLength filtering semantics#432

Merged
bartek-gralewicz merged 11 commits into
epic/1.0_breaking_changesfrom
bgralewicz/fix_history_length_semantics
Apr 30, 2026
Merged

fix: Fix historyLength filtering semantics#432
bartek-gralewicz merged 11 commits into
epic/1.0_breaking_changesfrom
bgralewicz/fix_history_length_semantics

Conversation

@bartek-gralewicz
Copy link
Copy Markdown
Contributor

@bartek-gralewicz bartek-gralewicz commented Apr 28, 2026

Description

Fixed a bug in historyLength filtering semantics. The behavior for undefined historyLength and equal to 0 were reversed.

Before

  • historyLength undefined => no history
  • historyLength = 0 => all history
  • historyLength = N => at most N most recent messages returned

After (spec aligned)

  • historyLength undefined => all history
  • historyLength = 0 => no history
  • historyLength = N => at most N most recent messages returned

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 28, 2026

🧪 Code Coverage

⬇️ Download Full Report

Base PR Delta
src/client/multitransport-client.ts 97.25% 97.24% 🔴 -0.01%
src/server/request_handler/default_request_handler.ts 81.09% 82.02% 🟢 +0.93%
src/server/store.ts 70.09% 69.3% 🔴 -0.79%
src/server/utils.ts 57.89% 63.63% 🟢 +5.74%
Total 87.69% 87.81% 🟢 +0.12%

Generated by coverage-comment.yml

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the DefaultRequestHandler to implement historyLength semantics per specification §3.2.4, allowing for full history retrieval when the parameter is undefined and empty history when it is zero or less. Corresponding unit tests have been added to cover these scenarios. The reviewer suggested creating a helper function in the test file to reduce code duplication when initializing task objects.

Comment thread test/server/default_request_handler.spec.ts Outdated
@bartek-gralewicz bartek-gralewicz marked this pull request as ready for review April 28, 2026 09:42
@bartek-gralewicz bartek-gralewicz requested a review from a team as a code owner April 28, 2026 09:42
Comment thread src/server/request_handler/default_request_handler.ts
Comment thread src/server/request_handler/default_request_handler.ts Outdated
Comment thread src/server/result_manager.ts Outdated
@bartek-gralewicz bartek-gralewicz merged commit fa0976a into epic/1.0_breaking_changes Apr 30, 2026
9 checks passed
@bartek-gralewicz bartek-gralewicz deleted the bgralewicz/fix_history_length_semantics branch April 30, 2026 10:10
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.

2 participants