Skip to content

feat: example of global memory#198

Open
Icemap wants to merge 1 commit intomainfrom
cheese/feat-demo-memory
Open

feat: example of global memory#198
Icemap wants to merge 1 commit intomainfrom
cheese/feat-demo-memory

Conversation

@Icemap
Copy link
Copy Markdown
Member

@Icemap Icemap commented Sep 29, 2025

No description provided.

@pingcap pingcap deleted a comment from chatgpt-codex-connector bot Oct 16, 2025
@pingcap pingcap deleted a comment from chatgpt-codex-connector bot Oct 16, 2025
@Mini256
Copy link
Copy Markdown
Member

Mini256 commented Oct 16, 2025

@codex Please review this pull request

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.

ℹ️ 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 review".

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

Codex can also answer questions or update the PR. Try commenting

Comment on lines +52 to +57
# Create tables if they don't exist
self.sessions_table = self._db.create_table(
schema=ChatSessionDB, if_exists="overwrite"
)
self.summaries_table = self._db.create_table(
schema=SessionSummaryDB, if_exists="overwrite"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid recreating TiDB tables on each startup

The TiDB connection builds chat_sessions and session_summaries with create_table(..., if_exists="overwrite"), which drops any existing data every time the app process starts. That behaviour erases all previously saved conversations and summaries, defeating the “global memory” goal and contradicting the comment that tables should be created only if missing. Using a non-destructive mode (e.g. append/ignore) or checking for existence would preserve data across runs.

Useful? React with 👍 / 👎.

Comment on lines +55 to +57
# Create session manager with temporary storage
session_manager = SessionManager(storage_path=temp_dir)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Update tests to match SessionManager signature

test_session_manager constructs SessionManager(storage_path=temp_dir), but SessionManager.__init__ no longer accepts a storage_path keyword. Executing the test suite immediately raises TypeError: SessionManager() got an unexpected keyword argument 'storage_path', so none of the session tests run. The tests need to be updated for the TiDB-backed implementation or the constructor should restore the expected parameter.

Useful? React with 👍 / 👎.

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