feat: auto save sessions before next user input#94
Conversation
| self.exchange = build_exchange(profile=load_profile(profile), notifier=self.notifier) | ||
| setup_logging(log_file_directory=LOG_PATH, log_level=log_level) | ||
|
|
||
| if name is not None and self.session_file_path.exists(): |
There was a problem hiding this comment.
extracted to _get_initial_messages function
|
so is this a fix to #89? |
|
could we merge #76 before we land this change (i don't have the ability to merge to |
Merged PR. Thanks for the heads-up! |
src/goose/cli/session.py
Outdated
| # rewind to right before the last user message | ||
| self.exchange.rewind() | ||
| print(traceback.format_exc()) | ||
| print(traceback.lformat_exc()) |
There was a problem hiding this comment.
Thanks for spotting this! 🙇
|
|
||
|
|
||
| def save_latest_session(file_path: Path, messages: List[Message]) -> None: | ||
| with tempfile.NamedTemporaryFile("w", delete=False) as temp_file: |
There was a problem hiding this comment.
nit: curious why going through the tempfile here? could do file_path.write_text
There was a problem hiding this comment.
using tempfile is to ensure atomic operation while writing file. It prevents from incomplete writes, losing original file dataduring the unexpected interruption or concurrency update
I guess the chance of unexpected scenario is small. However, the file could contain 100000 tokens (around 400KB). It would be safe to use tempfile to prevent from the unexpected behaviours
| user_input = self.prompt_session.get_user_input() | ||
| message = Message.user(text=user_input.text) if user_input.to_continue() else None | ||
|
|
||
| self.save_session() |
There was a problem hiding this comment.
im not totally sure this is better, but if the session has been saving as current_session, this is maybe a good time to change it's name using the old dialog? otherwise i think we'll be constantly overwriting sessions
There was a problem hiding this comment.
just saw the existing discussion in #89 - even better to generate a name at the start and save to that (but avoid current session which will cause overwrites)
Yes, it is related to. This PR contains 2 parts:
I was about to mention this issue in the PR, but forgot! Thanks for the reminder! |
|
@lifeizhou-ap so if I specify the session name as an arg, and I "exit" the prompt, no more duplicate prompting after this change? or do we still have that. |
There will be no more prompting at the end of the session. At the beginning of the session, we show a message about the location of the session file.
|
4 participants
Why
Currently we only save the session file when user exits the goose successfully. When user exits goose unexpectedly, the session file is not saved.
What
nameforsession start. If the name is not specified, use an auto-generated name as session nameNote
Added the log level option to
session resume, andsession planned. This is not relevant to this feature. It is a followup of #80