Ai Log Improvements #3412
Ai Log Improvements #3412
Conversation
* main: more tests clean up whitespace handling in peggy grammar Trying to fix comment duplication bug
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
|
I want to merge this to better check errors that we've been seeing. I think it's pretty safe, but comments/improvements later would be appreciated, of course. |
| message: `Step "${this.template.name}" started`, | ||
| }); | ||
|
|
||
| await this._run(); |
There was a problem hiding this comment.
Separation of concerns between run and _run is leaking here in several ways that make me think that _run should be inlined:
- "Step started" message above would happen even if
_rundecided not to run (seethis.state.kind !== "PENDING"check at the start of_run) - I don't think
this.state.kind !== "PENDING"check below, incompletionMessageconstruction, is needed —_runalways ends with non-pending state
| @@ -19,8 +18,6 @@ export function getLogEntryFullName(entry: LogEntry): string { | |||
| return "⚠️ Warning"; | |||
| case "error": | |||
| return "🚫 System Error"; | |||
| case "codeRunError": | |||
| return "❌ Code Run Error"; | |||
There was a problem hiding this comment.
Glas to see this entry type is gone
| @@ -97,7 +97,7 @@ export class LLMStepInstance<const Shape extends StepShape = StepShape> { | |||
| ) { | |||
| this.startTime = Date.now(); | |||
| this.id = crypto.randomUUID(); | |||
| this.logger = new Logger(); | |||
| this.logger = new Logger(this.workflow.id, this.workflow.getStepCount()); | |||
There was a problem hiding this comment.
I'll have to partially revert this for my upcoming PR - in it, LLMStepInstance is not parameterized with workflow, to avoid circular references, I pass the workflow in step.run() as a parameter instead.
But I see a way how to do that and keep the features you cared about here.
Closes #3411