Skip to content
This repository was archived by the owner on Jun 3, 2026. It is now read-only.

Commit 6c7e250

Browse files
afarntrogafarntrog
committed
cleanup comments
1 parent 9bed5c6 commit 6c7e250

1 file changed

Lines changed: 32 additions & 35 deletions

File tree

‎src/models/openai.ts‎

Lines changed: 32 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
277277
* ```
278278
*/
279279
async *stream(messages: Message[], options?: StreamOptions): AsyncIterable<ModelStreamEvent> {
280-
// Issue #1: Validate messages array is not empty
280+
// Validate messages array is not empty
281281
if (!messages || messages.length === 0) {
282282
throw new Error('At least one message is required')
283283
}
@@ -289,16 +289,16 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
289289
// Create streaming request with usage tracking
290290
const stream = await this._client.chat.completions.create(request)
291291

292-
// Track streaming state (Issue #1: Use mutable object for proper state tracking)
292+
// Track streaming state (Use mutable object for proper state tracking)
293293
const streamState = {
294294
messageStarted: false,
295295
textContentBlockStarted: false,
296296
}
297297

298-
// Track active tool calls for stop events (Issue #7, #8)
298+
// Track active tool calls for stop events
299299
const activeToolCalls = new Map<number, boolean>()
300300

301-
// Buffer usage to emit before message stop (Issue #10)
301+
// Buffer usage to emit before message stop
302302
let bufferedUsage: {
303303
type: 'modelMetadataEvent'
304304
usage: {
@@ -312,8 +312,7 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
312312
for await (const chunk of stream) {
313313
if (!chunk.choices || chunk.choices.length === 0) {
314314
// Handle usage chunk (no choices)
315-
// Issue #11: Add null checks for usage properties
316-
// Issue #10: Buffer usage to emit before message stop
315+
// Buffer usage to emit before message stop
317316
if (chunk.usage) {
318317
bufferedUsage = {
319318
type: 'modelMetadataEvent',
@@ -330,7 +329,7 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
330329
// Map chunk to SDK events
331330
const events = this._mapOpenAIChunkToSDKEvents(chunk, streamState, activeToolCalls)
332331
for (const event of events) {
333-
// Issue #10: Emit buffered usage before message stop
332+
// Emit buffered usage before message stop
334333
if (event.type === 'modelMessageStopEvent' && bufferedUsage) {
335334
yield bufferedUsage
336335
bufferedUsage = null
@@ -340,7 +339,7 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
340339
}
341340
}
342341

343-
// Emit any remaining buffered usage (Issue #10)
342+
// Emit any remaining buffered usage
344343
if (bufferedUsage) {
345344
yield bufferedUsage
346345
}
@@ -376,8 +375,8 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
376375
stream_options: { include_usage: true },
377376
}
378377

379-
// Issue #13: Add system prompt validation
380-
// Issue #17: Remove redundant type assertion
378+
// Add system prompt validation
379+
// Remove redundant type assertion
381380
if (options?.systemPrompt && options.systemPrompt.trim().length > 0) {
382381
request.messages.push({
383382
role: 'system',
@@ -406,7 +405,7 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
406405
request.presence_penalty = this._config.presencePenalty
407406
}
408407

409-
// Issue #16: Add tool specifications with validation
408+
// Add tool specifications with validation
410409
if (options?.toolSpecs && options.toolSpecs.length > 0) {
411410
request.tools = options.toolSpecs.map((spec) => {
412411
if (!spec.name || !spec.description) {
@@ -437,7 +436,7 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
437436
}
438437
}
439438

440-
// Issue #12: Validate that n=1 for streaming (if n is in params)
439+
// Validate that n=1 for streaming (if n is in params)
441440
if (this._config.params?.n && typeof this._config.params.n === 'number' && this._config.params.n > 1) {
442441
throw new Error('Streaming with n > 1 is not supported. Multiple choices cannot be streamed simultaneously.')
443442
}
@@ -481,7 +480,7 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
481480
})
482481
.join('')
483482

484-
// Issue #2: Validate content is not empty before adding
483+
// Validate content is not empty before adding
485484
if (contentText.trim().length > 0) {
486485
openAIMessages.push({
487486
role: 'user',
@@ -490,7 +489,7 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
490489
}
491490
}
492491

493-
// Issue #2: Validate message sequence for tool-only messages
492+
// Validate message sequence for tool-only messages
494493
if (toolResults.length > 0 && otherContent.length === 0) {
495494
// Having only tool results without user text is acceptable in OpenAI
496495
// The tool results will be added as separate tool messages below
@@ -500,7 +499,7 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
500499
for (const toolResult of toolResults) {
501500
if (toolResult.type === 'toolResultBlock') {
502501
// Format tool result content
503-
// Issue #14, #15, #19: Handle JSON serialization with context and consistent error handling
502+
// Handle JSON serialization with context and consistent error handling
504503
const contentText = toolResult.content
505504
.map((c) => {
506505
if (c.type === 'toolResultTextContent') {
@@ -510,7 +509,6 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
510509
return JSON.stringify(c.json)
511510
// eslint-disable-next-line @typescript-eslint/no-explicit-any
512511
} catch (error: any) {
513-
// Issue #15: Include data type information in error message
514512
const dataPreview =
515513
typeof c.json === 'object' && c.json !== null
516514
? `object with keys: ${Object.keys(c.json).slice(0, 5).join(', ')}`
@@ -522,15 +520,15 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
522520
})
523521
.join('')
524522

525-
// Issue #4: Validate content is not empty
523+
// Validate content is not empty
526524
if (!contentText || contentText.trim().length === 0) {
527525
throw new Error(
528526
`Tool result for toolUseId "${toolResult.toolUseId}" has empty content. ` +
529527
'OpenAI requires tool messages to have non-empty content.'
530528
)
531529
}
532530

533-
// Issue #5: Prepend error indicator if status is error
531+
// Prepend error indicator if status is error
534532
const finalContent = toolResult.status === 'error' ? `[ERROR] ${contentText}` : contentText
535533

536534
openAIMessages.push({
@@ -543,14 +541,13 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
543541
} else {
544542
// Handle assistant messages
545543
const toolUseCalls: unknown[] = []
546-
// Issue #21: Use array + join pattern for efficient string concatenation
544+
// Use array + join pattern for efficient string concatenation
547545
const textParts: string[] = []
548546

549547
for (const block of message.content) {
550548
if (block.type === 'textBlock') {
551549
textParts.push(block.text)
552550
} else if (block.type === 'toolUseBlock') {
553-
// Issue #14: Wrap JSON.stringify in try-catch
554551
try {
555552
toolUseCalls.push({
556553
id: block.toolUseId,
@@ -571,7 +568,7 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
571568
}
572569
}
573570

574-
// Issue #11: Trim text content to avoid whitespace-only messages
571+
// Trim text content to avoid whitespace-only messages
575572
const textContent = textParts.join('').trim()
576573

577574
const assistantMessage: { role: string; content: string; tool_calls?: unknown[] } = {
@@ -583,7 +580,7 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
583580
assistantMessage.tool_calls = toolUseCalls
584581
}
585582

586-
// Issue #3, #11: Only add if message has content or tool calls
583+
// Only add if message has content or tool calls
587584
if (textContent.length > 0 || toolUseCalls.length > 0) {
588585
openAIMessages.push(assistantMessage)
589586
} else {
@@ -627,17 +624,17 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
627624
): ModelStreamEvent[] {
628625
const events: ModelStreamEvent[] = []
629626

630-
// Issue #18: Use named constant for text content block index
627+
// Use named constant for text content block index
631628
const TEXT_CONTENT_BLOCK_INDEX = 0
632629

633-
// Issue #6: Validate choices array has at least one element
630+
// Validate choices array has at least one element
634631
if (!chunk.choices || chunk.choices.length === 0) {
635632
return events
636633
}
637634

638635
const choice = chunk.choices[0]
639636

640-
// Issue #6: Validate choice is an object
637+
// Validate choice is an object
641638
if (!choice || typeof choice !== 'object') {
642639
console.warn('Invalid choice format in OpenAI chunk:', choice)
643640
return events
@@ -668,7 +665,7 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
668665

669666
const delta = typedChoice.delta
670667

671-
// Issue #1: Handle message start (role appears) - update mutable state
668+
// Handle message start (role appears) - update mutable state
672669
if (delta?.role && !streamState.messageStarted) {
673670
streamState.messageStarted = true
674671
events.push({
@@ -677,9 +674,9 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
677674
})
678675
}
679676

680-
// Issue #2, #3: Handle text content delta with contentBlockIndex and start event
677+
// Handle text content delta with contentBlockIndex and start event
681678
if (delta?.content && delta.content.length > 0) {
682-
// Issue #3: Emit start event on first text delta
679+
// Emit start event on first text delta
683680
if (!streamState.textContentBlockStarted) {
684681
streamState.textContentBlockStarted = true
685682
events.push({
@@ -688,7 +685,7 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
688685
})
689686
}
690687

691-
// Issue #2: Include contentBlockIndex for text deltas
688+
// Include contentBlockIndex for text deltas
692689
events.push({
693690
type: 'modelContentBlockDeltaEvent',
694691
contentBlockIndex: TEXT_CONTENT_BLOCK_INDEX,
@@ -702,13 +699,13 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
702699
// Handle tool calls
703700
if (delta?.tool_calls && delta.tool_calls.length > 0) {
704701
for (const toolCall of delta.tool_calls) {
705-
// Issue #9: Validate tool call index
702+
// Validate tool call index
706703
if (toolCall.index === undefined || typeof toolCall.index !== 'number') {
707704
console.warn('Received tool call with invalid index:', toolCall)
708705
continue
709706
}
710707

711-
// Issue #9: Validate index is non-negative and reasonable
708+
// Validate index is non-negative and reasonable
712709
if (toolCall.index < 0 || toolCall.index > 100) {
713710
console.warn(`Received tool call with out-of-range index: ${toolCall.index}`, toolCall)
714711
continue
@@ -725,7 +722,7 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
725722
toolUseId: toolCall.id,
726723
},
727724
})
728-
// Issue #7, #8: Track active tool calls
725+
// Track active tool calls
729726
activeToolCalls.set(toolCall.index, true)
730727
}
731728

@@ -745,7 +742,7 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
745742

746743
// Handle finish reason (message stop)
747744
if (typedChoice.finish_reason) {
748-
// Issue #4: Emit stop event for text content if it was started
745+
// Emit stop event for text content if it was started
749746
if (streamState.textContentBlockStarted) {
750747
events.push({
751748
type: 'modelContentBlockStopEvent',
@@ -754,7 +751,7 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
754751
streamState.textContentBlockStarted = false
755752
}
756753

757-
// Issue #7, #22: Emit stop events for all active tool calls and delete during iteration
754+
// Emit stop events for all active tool calls and delete during iteration
758755
for (const [index] of activeToolCalls) {
759756
events.push({
760757
type: 'modelContentBlockStopEvent',
@@ -771,7 +768,7 @@ export class OpenAIModel implements Model<OpenAIModelConfig, ClientOptions> {
771768
content_filter: 'contentFiltered',
772769
}
773770

774-
// Issue #13: Log unknown stop reasons
771+
// Log unknown stop reasons
775772
let stopReason = stopReasonMap[typedChoice.finish_reason]
776773
if (!stopReason) {
777774
const fallbackReason = this._snakeToCamel(typedChoice.finish_reason)

0 commit comments

Comments
 (0)