-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix: last_code_generated retrieval logic in multi-turn conversations #1784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: last_code_generated retrieval logic in multi-turn conversations #1784
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Changes requested ❌
Reviewed everything up to 3180cc6 in 1 minute and 45 seconds. Click for details.
- Reviewed
61lines of code in3files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. pandasai/core/code_generation/base.py:38
- Draft comment:
Good update using the cleaned code to update last_code_generated. Consider whether the initial assignment of last_code_generated (before cleaning) is necessary, as it gets immediately overwritten by the cleaned version. If the raw code isn't needed later, removing the redundant assignment could reduce confusion. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While the comment identifies a potential redundancy, we don't have enough context to know if the raw code needs to be temporarily stored for logging or debugging purposes. The initial assignment happens before logging the raw code, so it may serve a purpose. Without understanding the full usage of last_code_generated throughout the codebase, removing it could be risky. The comment makes assumptions about the usage of last_code_generated without full context. The initial assignment might be intentional for logging or debugging purposes. Given that the raw code is logged immediately after the first assignment, the initial assignment may serve a purpose in the logging flow. The comment should be deleted as it makes assumptions without sufficient context and the current implementation may be intentional for logging purposes.
2. pandasai/core/prompts/__init__.py:20
- Draft comment:
Retrieving last_code_generated directly from the context attribute (instead of using context.get(...)) is a clear improvement for multi-turn conversation continuity. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply states an opinion about the code change without offering any specific guidance or questions for the PR author.
Workflow ID: wflow_shanL1yxu5U6Xrmq
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
tests/unit_tests/agent/test_agent.py
Outdated
|
|
||
| def test_last_code_generated_retrieval(self, agent: Agent): | ||
| """Test that last_code_generated is correctly retrieved in get_chat_prompt_for_sql.""" | ||
| # 设置 last_code_generated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new test for last_code_generated retrieval is thorough. Consider using a consistent language in comments (currently mixing Chinese and English) to improve clarity and maintainability.
| # 设置 last_code_generated | |
| # Set last_code_generated |
This comment was generated because it violated a code review rule: mrule_kO2c9rTcwkbqZtg4.
|
Hi @ArslanSaleem , could you please take a look at this PR when you have time? Thanks! |
|
Hi there, I've looked into the logs for the failed CI check. The failure seems to be caused by an This seems to be a transient network issue within the CI environment and is unrelated to the changes in this PR. Since I don't have permissions to re-run the job, could a maintainer please help restart the check? Thanks! |
Fix last_code_generated retrieval logic in multi-turn conversations
Issue Description
In multi-turn conversations, previously generated code was not being correctly passed to new code generation processes, causing a lack of continuity in code generation. This issue affected the user experience when conducting multi-step analyses that required building upon previous code.
Root Cause
The issue stemmed from a mismatch in how
last_code_generatedwas stored and retrieved:The generated code was stored in the
AgentState.last_code_generatedattribute:However, when creating a new prompt, the code attempted to retrieve it from the
intermediate_valuesdictionary:The
AgentState.getmethod only fetches values from theintermediate_valuesdictionary:This disconnect meant that in multi-turn conversations, each new code generation would start from scratch rather than building upon previous code.
Changes Made
This PR fixes the issue by:
Modifying
get_chat_prompt_for_sqlto directly use thelast_code_generatedattribute:Ensuring the cleaned code is also stored in
last_code_generated:Adding a test case to verify the fix:
Benefits
This fix ensures:
Testing
A new test case
test_last_code_generated_retrievalwas added to verify the fix, which passes successfully.Important
Fixes
last_code_generatedretrieval logic in multi-turn conversations to ensure code continuity.last_code_generatedretrieval inget_chat_prompt_for_sqlto usecontext.last_code_generatedinstead ofcontext.get().last_code_generatedingenerate_code()inbase.py.test_last_code_generated_retrievalintest_agent.pyto verify correct retrieval oflast_code_generated.This description was created by
for 3180cc6. You can customize this summary. It will automatically update as commits are pushed.