Skip to content

feat(llm): configurable first-token timeout#212

Open
lin-snow wants to merge 9 commits into
mainfrom
feat/llm-first-token-timeout
Open

feat(llm): configurable first-token timeout#212
lin-snow wants to merge 9 commits into
mainfrom
feat/llm-first-token-timeout

Conversation

@lin-snow

@lin-snow lin-snow commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds an opt-in, per-node first_token_timeout and threads it through graphon's LLM invoke path. graphon carries the value from node config down to the LLMProtocol invoke calls — it does not enforce the timeout. Enforcement (closing the connection when the first token is late, then raising FirstTokenTimeoutError) is the host transport adapter's job (e.g. Dify's ModelInstance), delivered separately.

How it fits together

flowchart LR
    subgraph G["graphon — this PR (carrier only)"]
        direction TB
        CFG["LLM-family node data:<br/>invocation.first_token_timeout (ms, 0 = disabled)"]
        NODE["LLM / QuestionClassifier /<br/>ParameterExtractor node"]
        PROTO["LLMProtocol.invoke_llm(<br/>..., first_token_timeout=...)"]
        CFG --> NODE --> PROTO
    end
    subgraph H["host adapter — e.g. Dify (separate, later phase)"]
        direction TB
        ADP["ModelInstance / DifyPreparedLLM"]
        GATE{"first token<br/>within timeout?"}
        ADP --> GATE
        GATE -->|"yes"| OK["stream normally"]
        GATE -->|"no"| ERR["raise FirstTokenTimeoutError"]
    end
    PROTO -->|"protocol call — contract boundary"| ADP
    ERR -.->|"bubbles up as node failure"| STRAT["node error-strategy:<br/>retry / fail-branch / default"]
Loading

Changes

File Change
model_runtime/errors/invoke.py New FirstTokenTimeoutError(InvokeConnectionError) — canonical kernel type for hosts to raise/import. Defined here, not raised by graphon (carrier only).
nodes/llm/entities.py New LLMInvocationConfig value object: first_token_timeout: int = Field(default=0, ge=0) (ms; 0 = disabled, negatives rejected at validation) + first_token_timeout_seconds property (maps 0 to None). Composed onto the LLM / question-classifier / parameter-extractor node data as an invocation field — same idiom as ModelConfig / VisionConfig. RetryConfig / base_node_data.py are untouched — the deadline is a client-side invocation concept, not a retry one, and must not sit on the shared base that every node inherits.
nodes/llm/runtime_protocols.py Optional keyword-only first_token_timeout: float | None = None on LLMProtocol.invoke_llm and invoke_llm_with_structured_output.
nodes/llm/node.py LLMNode.invoke_llm forwards the value; _invoke_llm_for_run reads self.node_data.invocation.first_token_timeout_seconds (non-polling path).
nodes/question_classifier/…, nodes/parameter_extractor/… Node data composes LLMInvocationConfig; each node reads invocation.first_token_timeout_seconds and passes it through.
dsl/slim/llm.py SlimLLM accepts the parameter for protocol conformance; does not enforce it (v1 standalone/DSL limitation).

Compatibility & release ordering ⚠️

Backward compatible at graphon's config / serialization layer: workflows saved before this field deserialize fine (invocation defaults to a config with first_token_timeout = 0 = disabled; no migration).

It is not transparent at the host call contract. LLMNode.invoke_llm passes first_token_timeout= unconditionally (even when unset, it passes None). Any LLMProtocol implementation that lacks the parameter and lacks **kwargs raises TypeError on every LLM call — not only when a timeout is configured.

graphon (caller) host LLMProtocol impl Result
old (never passes the kwarg) old or new ✅ works
new (this PR) old — no param, no **kwargs TypeError on every LLM / QC / PE invoke
new (this PR) new — accepts the kwarg ✅ works

Ordering requirement: the host adapter must accept first_token_timeout (accept-and-ignore is enough) before the host bumps its graphon pin to a version containing this PR. graphon's own merge/release is independent — the only gate is the host's pin bump.

For Dify specifically: DifyPreparedLLM (api/core/workflow/node_runtime.py) currently has neither the parameter nor **kwargs and pins graphon==0.6.0, so its accept-and-ignore shim must land before Dify bumps the pin.

flowchart LR
    A["host: adapter accepts kwarg<br/>(accept-and-ignore shim)"] --> B["host: bump graphon pin<br/>to a version with this PR"]
    B --> C["host: implement real enforcement<br/>+ raise FirstTokenTimeoutError"]
Loading

Out of scope

Transport-level enforcement; the lower LLMModelRuntime protocol (not on the node invoke path); the LLM polling branch (start_llm_polling has no such parameter — intentionally not threaded, pinned by a test).

Tests

LLMInvocationConfig defaults / ms→s conversion / 0-disables / a negative value rejected at validation (ge=0), plus a guard asserting the three LLM-family node data classes compose an invocation field and that the deadline does not leak onto RetryConfig / BaseNodeData; forwarding assertions covering both a configured value and the default None for the LLM, question-classifier, and parameter-extractor nodes (question-classifier and parameter-extractor drive it end-to-end through model_validate({"invocation": {...}})); a test pinning that the polling path does not receive the parameter. just check (ruff + ty) and the full suite (618 tests) pass.

@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. enhancement New feature or request labels Jul 8, 2026
@lin-snow lin-snow self-assigned this Jul 8, 2026
lin-snow added 3 commits July 8, 2026 23:28
Canonical connection-error subtype raised when an LLM does not return its
first token within a configured first_token_timeout. Defined in the kernel
so host transport adapters share one exception type.
Add an opt-in per-node first_token_timeout (milliseconds, 0 = disabled)
plus a first_token_timeout_seconds property that maps 0 to None so a
missing/disabled value flows down as 'no gate'. Inherited by every node
via BaseNodeData.retry_config; backward compatible with workflows
serialized before the field existed.
Add an optional first_token_timeout to LLMProtocol.invoke_llm and its
structured-output variant, and forward it from the LLM, question-classifier
and parameter-extractor nodes (read from retry_config). graphon only carries
the value; enforcement is the host transport adapter's responsibility. The
built-in SlimLLM runtime accepts the parameter for protocol conformance but
does not enforce it.
@lin-snow lin-snow force-pushed the feat/llm-first-token-timeout branch from 069a2e9 to 5895fe4 Compare July 8, 2026 15:29
- question-classifier: assert the default (unset retry_config) forwards
  first_token_timeout=None, matching the LLM and parameter-extractor tests.
- LLM polling path: pin that start_llm_polling does not receive
  first_token_timeout, so a future change that threads it there is flagged
  as out-of-scope for v1.
lin-snow added 2 commits July 9, 2026 16:08
first_token_timeout is a client-side invocation deadline, not a retry
concept. Keeping it on the shared RetryConfig (which BaseNodeData attaches
to every node) polluted non-LLM nodes with an LLM-only field and conflated
'deadline' with 'retry container'.

Introduce a FirstTokenTimeoutConfig mixin in nodes/llm/entities.py and mix
it into the LLM, question-classifier, and parameter-extractor node data;
RetryConfig / base_node_data.py are left untouched. Nodes now read
self.node_data.first_token_timeout_seconds directly.
@lin-snow lin-snow marked this pull request as draft July 9, 2026 08:21
lin-snow added 3 commits July 9, 2026 16:24
Inheriting a config mixin onto node data overstated the relationship (has-a,
not is-a) and broke the codebase idiom, where shared LLM config (ModelConfig,
VisionConfig) is composed as fields rather than inherited. Declare
first_token_timeout as a plain int field on each of the LLM / question-classifier
/ parameter-extractor node data, and convert ms->seconds via a shared
first_token_timeout_seconds() helper at the invoke call sites.
…onfig

Replace the free-function helper with an LLMInvocationConfig value object
(field + ms->seconds property), composed onto the LLM / question-classifier /
parameter-extractor node data as an 'invocation' field — the same
composition idiom the codebase uses for ModelConfig / VisionConfig. Nodes
read self.node_data.invocation.first_token_timeout_seconds; no shared
function or mixin. DSL nests as 'invocation: { first_token_timeout: <ms> }'.
A negative value is a typo, not an intentional disable (0 is the disable
sentinel). Without a bound it silently degrades to "no timeout" via the
seconds property. Add ge=0 so the misconfiguration fails loudly at
model validation instead.
@lin-snow lin-snow changed the title feat(llm): configurable first-token timeout (contract + wiring) feat(llm): configurable first-token timeout Jul 9, 2026
@lin-snow lin-snow marked this pull request as ready for review July 9, 2026 09:28
@lin-snow lin-snow requested review from WH-2099 and laipz8200 and removed request for WH-2099 July 9, 2026 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant