Skip to content

test(checkpoint): cover InMemorySaver before and limit listing#7235

Open
ycc (yangcongcong-coding) wants to merge 2 commits intolangchain-ai:mainfrom
yangcongcong-coding:memory-list-before-limit
Open

test(checkpoint): cover InMemorySaver before and limit listing#7235
ycc (yangcongcong-coding) wants to merge 2 commits intolangchain-ai:mainfrom
yangcongcong-coding:memory-list-before-limit

Conversation

@yangcongcong-coding
Copy link
Copy Markdown

Description:\nAdds missing before/limit coverage for InMemorySaver listing in both sync and async tests. The new assertions keep the checks thread-scoped so they verify the current in-memory listing semantics without depending on cross-thread ordering.\n\nIssue: N/A\n\nDependencies: None\n\nTwitter handle: @yangcongcongcc

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 283221582a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

Comment on lines +142 to +145
search_results_6 = list(
self.memory_saver.list(
{"configurable": {"thread_id": "thread-2"}},
before=search_results_5[1].config,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Select the before cursor deterministically in list tests

The new before coverage depends on search_results_5[1] being the inner namespace, but the preceding assertion explicitly treats search_results_5 as unordered by comparing a set of namespaces. InMemorySaver.list() currently iterates namespaces in insertion order, so a harmless refactor that changes that order would make this cursor point at the root checkpoint instead and turn the expected one-row result into an empty list even though before still works. Please choose the cursor by checkpoint_ns (or sort first) rather than by positional index; the same issue is duplicated in test_asearch.

Useful? React with 👍 / 👎.

@github-actions
Copy link
Copy Markdown
Contributor

This PR has been automatically closed because it does not link to an approved issue.

All external contributions must reference an approved issue or discussion. Please:

  1. Find or open an issue describing the change
  2. Wait for a maintainer to approve and assign you
  3. Add Fixes #<issue_number>, Closes #<issue_number>, or Resolves #<issue_number> to your PR description and the PR will be reopened automatically

Maintainers: reopen this PR or remove the missing-issue-link label to bypass this check.

@github-actions github-actions Bot closed this Mar 24, 2026
@mdrxy Mason Daugherty (mdrxy) added bypass-issue-check Maintainer override: skip issue-link enforcement and removed missing-issue-link labels Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bypass-issue-check Maintainer override: skip issue-link enforcement external

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants