feat/odedgo/atp-checkpointer #54
Conversation
| const parsed = OperationCheckpointManager.parseCheckpointId(fullCheckpointId); | ||
|
|
||
| if (!parsed) { | ||
| // If not a full ID format, try loading from current execution (backwards compatibility) |
There was a problem hiding this comment.
What backward compatibility is needed?
| return this.previewObject(result as Record<string, unknown>); | ||
| } | ||
|
|
||
| if (typeof result === 'string' && result.length > 100) { |
There was a problem hiding this comment.
For string the preview size is hard coded ( and not consistent - in other places its 50 ). Doesn't it make more sense to just use config.previewSize for strings as well?
| /** | ||
| * Default configuration values | ||
| */ | ||
| export const DEFAULT_CHECKPOINT_CONFIG: Required<Omit<CheckpointConfig, 'strategy'>> = { |
There was a problem hiding this comment.
Move from types to consts file
| lines.push('In your next code iteration, you can:'); | ||
| lines.push('1. Directly use data returned from saved checkpoints (full_snapshot)'); | ||
| lines.push('2. Restore a checkpoint value programmatically using:'); | ||
| lines.push(' const value = await __restore.checkpoint("<checkpoint_id>");'); |
There was a problem hiding this comment.
Maybe define '__restore' and 'checkpoint' as consts, and reuse them when adding it to the runtime global at packages/server/src/executor/sandbox-injector.ts
There was a problem hiding this comment.
Tho its up to the LLM to choose from, ur suggestion always "injecting" them ?
There was a problem hiding this comment.
Just ment that '__restore' and 'checkpoint' won't be hardcoded in the prompt, since it will break stuff.
You can do something like
const RESTORE_OPERATION = '__restore';
lines.push(`...${RESTORE_OPERATIPN}`)
and in sandbox-injector.js
checkpointSetup += `
globalThis.${RESTORE_OPERATION} = {
...
}
};`;
( same for checkpointer )
Main changes