Skip to content

log executeCode return values - #300

Open
maxwellpeterson wants to merge 3 commits into
mainfrom
mpeterson/execute-code-return-values
Open

log executeCode return values#300
maxwellpeterson wants to merge 3 commits into
mainfrom
mpeterson/execute-code-return-values

Conversation

@maxwellpeterson

@maxwellpeterson maxwellpeterson commented Aug 21, 2026

Copy link
Copy Markdown
Member

Kimi likes to return values when using the executeCode tool instead of logging them. Return values are currently discarded, which causes confusion. We can automatically log them instead.

This is probably a better alternative to #299

Fixes #209

@github-actions github-actions Bot added kernel Changes to the Workshop kernel gatekeeper Changes to a gatekeeper integration labels Aug 21, 2026
@github-actions

Copy link
Copy Markdown

Preview: pr300-mpeterson-exe-a7d98d39

https://pr300-mpeterson-exe-a7d98d39-router.cloudflare-os-previews.workers.dev

Dashboard · deleted when this PR closes

@maxwellpeterson
maxwellpeterson marked this pull request as ready for review August 21, 2026 19:28
Comment thread packages/workshop-backend/src/overseer.ts Outdated
}
await agent(self, env, this.ctx);
let result = await agent(self, env, this.ctx);
if (result !== undefined) console.log(result);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we prefix this with some text explaining it is the return value of the function, to disambiguate from any logs appearing before that?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added a "Return value: " prefix in d954141

Comment thread packages/workshop-backend/src/agent.ts Outdated
Comment on lines +283 to +285
* Return or print the schedule ID so it appears in the `executeCode` output. Empty output is not
* evidence that registration failed, and the disabled hook appearing in Connections confirms
* success. Do not retry solely because no ID was printed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This text is weird. Why are we explaining the UI to the agent?

Did we actually have problems with agents getting confused and thinking that schedules had failed to be registered? If so, is the solution perhaps to simply emphasize that if the call doesn't throw, then the schedule has been registered?

Why does an agent care about the schedule ID at all? What can it do with this? Why are we telling it to log or return it?

@ndisidore ndisidore Aug 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The schedule id can be used by the agent to confirm the schedule exists via list()
Non-SOTA models would often get confused because they would call executeCode but to them nothing happened - it would just get a void output.
The would interpret this as a fail and attempt to re-register (often many times)

This is probably less important now that it won't be a void return, but returning the schedule id may still be useful so it knows how to check if it is still enabled.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When you experienced this, were those non-SOTA models specifically trying to return the schedule ID? If so, maybe simply the fact that we're logging now will mean that they no longer get confused, without the need for this text?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Another thing we could do is: If the executeCode logs nothing, we could write "(function succeeded with no output)"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gatekeeper Changes to a gatekeeper integration kernel Changes to the Workshop kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

executeCode can drop module return values

3 participants