Repository navigation
fix(tools): require timezone on time tools, inject user timezone into the prompt #257
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
base: main
Are you sure you want to change the base?
Changes from all commits
b449c3b
5b0730a
ee5df89
1e1445a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
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.
wdyt of passing just the date and the timezone since after some time in the chat session or picking it up again, the time would be off, so would the date but that wouldn't affect that many sessions, only the ones that span across days.
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.
Not a concern, substituted inside call_model on every LLM round (and every turn of a resumed session), recomputes the current date/time anew
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.
ah you're right, damn, but that's all the more reason to not include the time in the system prompt, it would bust the cache every second.
what do you think about adding a new tool for "date + time + user's timezone" instead?
(for later reference: https://github.com/nextcloud/context_agent/blob/ee5df89e3dcb71f05d2665647adf27d023f7fa96/ex_app/lib/agent.py #L212-L218)
Uh oh!
There was an error while loading. Please reload this page.
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.
Did some digging: prefix caching needs byte identical prefixes, and dynamic timestamps in system prompts are a known cache killer in production, e.g. openclaw hit exactly this and paid 10x (openclaw#19534), the date itself is byte stable within a day though, so only the current time breaks the cache on every call.
Claude.ai itself uses "current date in the system prompt, injected once per conversation" (Claude system prompt docs, Claude 4 prompt analysis) plus an official time MCP reference server for precise time (modelcontextprotocol/servers/src/time), and the general rule is stable content before variable content with cache points after the stable part (AWS Bedrock prompt caching).
So i'd follow that pattern: keep the date in the prompt, drop the time, add a small
get_current_datetimetool. "tomorrow at noon" needs the date and the tz, not the minute.