fix(generate-schedule): switch to streaming AI path to avoid body-read abort - #515

Enforce remaining budget before starting next model call

runScheduleModelChain only checks elapsed > budgetMs before each attempt, but each callModelWithStreaming attempt can still block for up to 90s. With budgetMs: 130_000 in generate-schedule, a first slow attempt near 90s can still start a second attempt and run past Supabase’s ~150s hard limit, causing the function to be killed before it returns any response. This reintroduces the timeout-kill failure mode the budget is meant to prevent.

Useful? React with 👍 / 👎.

Addressed in commit f51db0e. runScheduleModelChain now passes a per-attempt AbortSignal set to fire after remaining = budgetMs - (now() - start), combined with the existing 90s ceiling via AbortSignal.any. When the chain budget elapses mid-call, the streaming fetch is aborted and the runner breaks (does not start the next model). New tests cover both the in-flight-abort and the insufficient-remaining-time-before-next-attempt cases.

Reject streaming responses that end due to token limit

The new streaming path accepts the first JSON-parseable payload, but it no longer checks whether generation stopped with finish_reason === "length" (the previous non-streaming loop explicitly skipped truncated outputs). Since callStreaming returns only text, a token-limited completion that still parses as JSON can now be treated as success, returning an incomplete schedule instead of falling back to the next model.

Useful? React with 👍 / 👎.

Addressed in commit f51db0e. processStreamedResponse now captures finish_reason from the last delta; callModelWithStreaming returns null on finish_reason === 'length', restoring the previous non-streaming loop's truncation behaviour so the chain falls through to the next model.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Remove production console.log in model-attempt logging.

Use a non-console logging path or gate this behind a dev-only condition; this log was newly added and violates the TS logging rule.

As per coding guidelines, "NEVER leave console.logs in production code - remove or gate with import.meta.env.DEV check".

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@supabase/functions/_shared/schedule-ai-runner.ts` around lines 68 - 70, The
console.log call that prints `[${edgeFunction}] Trying model: ${model.name}
(streaming, remaining=${remaining}ms)` must be removed from production; either
replace it with a proper logger call (e.g., processLogger.debug or
processLogger.info) using the same context (edgeFunction, model.name, remaining)
or gate the existing message behind a dev-only check (if import.meta.env.DEV) so
it never runs in production; update the model-attempt logging in the same block
where edgeFunction, model, and remaining are available (the current console.log)
to use the chosen non-console path.

Declining. The import.meta.env.DEV guard is a Vite/frontend convention — Deno's import.meta doesn't expose .env, so the suggested gate cannot run in this file. The 70+ Supabase edge functions in this repo (including _shared/streaming.ts, _shared/ai-caller.ts, process-receipt, process-bank-statement) consistently use console.* as the canonical observability sink — those logs land in Supabase Edge Function Logs, which is exactly how this PR's root cause was diagnosed. This specific log (which model the chain is attempting, with remaining-budget annotation) is intentional production observability for the new budget-enforcement path.

@jdelgado2002, understood — thanks for the thorough context. You're right that import.meta.env.DEV is a Vite-specific convention with no equivalent in Deno, and given that console.* is the established, intentional observability pattern across all edge functions in this repo (landing directly in Supabase Edge Function Logs), this log is entirely appropriate. I'll withdraw the suggestion.

(ᵔᴥᵔ) 🐇

Learnt from: jdelgado2002
Repo: toyiyo/nimble-pnl PR: 515
File: supabase/functions/_shared/schedule-ai-runner.ts:68-70
Timestamp: 2026-05-23T14:20:10.774Z
Learning: In this repository's Deno/Supabase Edge Functions (under `supabase/functions/`), `console.log`, `console.warn`, and `console.error` are the canonical and intentional observability sink — logs surface directly in Supabase Edge Function Logs. Do NOT flag `console.*` calls in these files as violations; the frontend `import.meta.env.DEV` guard does not apply here and cannot run in the Deno runtime.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

#!/bin/bash
set -euo pipefail

FILE="supabase/functions/_shared/streaming.ts"

# Ensure file exists
ls -la "$FILE"

# Show relevant sections around the reported line numbers
sed -n '80,140p' "$FILE" | cat -n

echo "----"
sed -n '140,220p' "$FILE" | cat -n

echo "----"
# Grep for any console.log usage in this file
rg -n "console\.log" "$FILE" || true

echo "----"
# Grep for explicit any typing in this file
rg -n ":\s*any\b|any\s*\)" "$FILE" || true

#!/bin/bash
set -euo pipefail

FILE="supabase/functions/_shared/streaming.ts"

ls -la "$FILE"
sed -n '80,140p' "$FILE" | cat -n
echo "----"
sed -n '140,220p' "$FILE" | cat -n
echo "----"
rg -n "console\.log" "$FILE" || true
echo "----"
rg -n ":\s*any\b|any\s*\)" "$FILE" || true

FILE="supabase/functions/_shared/streaming.ts"
sed -n '95,120p' "$FILE"
sed -n '155,180p' "$FILE"
rg -n "console\.log" "$FILE" || true
rg -n "requestBody\s*:\s*any|:\s*any\b" "$FILE" || true

ls -la supabase/functions/_shared || true
ls -la supabase/functions/_shared/streaming.ts || true
nl -ba supabase/functions/_shared/streaming.ts | sed -n '95,120p'

Fix TypeScript any typing and remove production console.logs in callModelWithStreaming.

• Replace requestBody: any with a concrete request payload type (or a temporary typed boundary like Record<string, unknown>) and narrow where fields such as messages, temperature, and max_tokens are used.
• The added console.log calls (e.g., attempt/start/completed and rate-limit logs) violate the “no console.logs in production” guideline—remove them or gate them behind import.meta.env.DEV (or route through the existing logger utility).

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@supabase/functions/_shared/streaming.ts` around lines 103 - 109, The function
callModelWithStreaming currently uses requestBody: any and emits production
console.log statements; change requestBody to a safer type such as
Record<string, unknown> (or a concrete interface) and narrow-access fields where
used (e.g., cast/validate requestBody.messages as Message[], and types for
temperature and max_tokens before use) so TypeScript catches misuse, and remove
all direct console.log calls in callModelWithStreaming (replace with the project
logger or wrap them with import.meta.env.DEV guards) including the
attempt/start/completed and rate-limit logs; update any helper variables or type
assertions inside callModelWithStreaming to reflect the new type and perform
runtime checks where necessary.

Declining both items. (1) requestBody: any predates this PR — it's on the existing callModelWithStreaming signature; tightening it touches callAIWithFallbackStreaming and every edge function that constructs an OpenRouter request body, out of scope for this hotfix. (2) For the console.log concern, see reply on schedule-ai-runner.ts:70 — import.meta.env.DEV is Vite-only and the codebase convention for Deno edge functions is console.* straight to Supabase Edge Function Logs.

@jdelgado2002, understood on both counts — acknowledged and won't flag these again.

Noted for future reviews:

• requestBody: any is a pre-existing signature shared across edge functions; tightening it requires a broader refactor touching callAIWithFallbackStreaming and is intentionally deferred.
• Deno edge functions on Supabase use console.* directly (routed to Supabase Edge Function Logs); import.meta.env.DEV is Vite-only and does not apply here.

🐇 _(--)/

Learnt from: jdelgado2002
Repo: toyiyo/nimble-pnl PR: 515
File: supabase/functions/_shared/streaming.ts:103-109
Timestamp: 2026-05-23T14:20:15.967Z
Learning: In the toyiyo/nimble-pnl repository, `requestBody: any` in `callModelWithStreaming` (supabase/functions/_shared/streaming.ts) and related helpers like `callAIWithFallbackStreaming` is a pre-existing type that is intentionally broad because it is shared across all OpenRouter edge functions. Tightening it requires a separate, broader refactor and should not be flagged as a hotfix issue.

Learnt from: jdelgado2002
Repo: toyiyo/nimble-pnl PR: 515
File: supabase/functions/_shared/streaming.ts:103-109
Timestamp: 2026-05-23T14:20:15.967Z
Learning: In the toyiyo/nimble-pnl repository, Deno edge functions (under supabase/functions/) use `console.log`, `console.warn`, and `console.error` directly — this is the project convention, as output routes to Supabase Edge Function Logs. `import.meta.env.DEV` guards are Vite-only and do not apply to Deno/Supabase edge functions. Do not flag bare `console.*` calls in these files.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Drop the new production console.log for stream completion.

This newly added log should be removed or dev-gated to keep production logging policy compliant.

As per coding guidelines, "NEVER leave console.logs in production code - remove or gate with import.meta.env.DEV check".

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@supabase/functions/_shared/streaming.ts` around lines 163 - 165, Remove the
new unguarded console.log in streaming.ts (the log that prints
`${modelConfig.name} stream completed...`) or wrap it behind a dev-only guard;
specifically either delete the console.log call inside the stream completion
block or condition it with import.meta.env.DEV (or your project's equivalent DEV
flag) so it only runs in development. Locate the console.log that references
modelConfig.name, content.length and finishReason and update it accordingly to
comply with the "no console.log in prod" guideline.

Declining. This log was already present in the file before this PR (prior text: ✅ ${modelConfig.name} stream completed. Content length: ${content.length}); this commit only extended it to include finish_reason so we can verify the new truncation-detection path in production logs. Same Deno-edge-function reasoning as schedule-ai-runner.ts:70.

@jdelgado2002, understood — the log predates this PR and the extension adds meaningful observability for the new finish_reason=length detection path in production. Acknowledged and won't flag pre-existing logs extended for diagnostic purposes in this Deno edge-function context.

(ᴗ_ ᴗ。)

Learnt from: jdelgado2002
Repo: toyiyo/nimble-pnl PR: 515
File: supabase/functions/_shared/streaming.ts:163-165
Timestamp: 2026-05-23T14:20:16.991Z
Learning: In the `supabase/functions/_shared/streaming.ts` Deno edge-function file (and related edge functions in this repo), production `console.log` statements are intentional for observability. Pre-existing logs that are extended to surface new diagnostic fields (e.g., adding `finish_reason` to an existing stream-completion log) are acceptable and should not be flagged as guideline violations. The "no console.log in prod" guideline applies to newly introduced debug logs, not to extending pre-existing operational logs in Deno edge functions.