Skip to content

fix: agent time out issue#303

Merged
yanmxa merged 1 commit into
genai-io:mainfrom
hchenxa:timeoutissue
Jul 13, 2026
Merged

fix: agent time out issue#303
yanmxa merged 1 commit into
genai-io:mainfrom
hchenxa:timeoutissue

Conversation

@hchenxa

@hchenxa hchenxa commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

What & why

#302

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Docs
  • Other:

Testing

GOCACHE=/private/tmp/san-go-build-cache go test ./...

Checklist

  • Follows Conventional Commits (feat:, fix:, docs:, …)
  • All commits signed off (git commit -s) — certifies the DCO
  • Tests pass locally
  • Docs updated (if behavior or config changed)
  • Read and follow the Code of Conduct

@san-ci san-ci Bot added the dco:yes label Jul 12, 2026
@san-ci

san-ci Bot commented Jul 12, 2026

Copy link
Copy Markdown

[REVIEW NOTIFIER] This PR requires a review from @yanmxa.

Once this PR has been reviewed and has the lgtm label, it will be automatically squash-merged.

Suggested reviewer

Based on OWNERS, @yanmxa is suggested to review this PR.

How to get your PR reviewed:

  • Address reviewer feedback and push changes
  • Reviewer adds lgtm label when satisfied
  • An approving review + lgtm will trigger auto-merge

@san-ci san-ci Bot added the approved label Jul 12, 2026
@hchenxa

hchenxa commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

/assign @yanmxa

@san-ci san-ci Bot assigned yanmxa Jul 12, 2026
@san-ci

san-ci Bot commented Jul 12, 2026

Copy link
Copy Markdown

@hchenxa assigned @yanmxa to this PR.

@yanmxa yanmxa left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two concerns:

  1. Default 60s → 180s weakens an intentionally tight signal. FirstChunkTimeout = 5m already handles reasoning models thinking before the first token; once streaming, both DeepSeek and Claude extended thinking emit reasoning as chunks, so 60s of silence is usually a real stall. #302 has no repro. Prefer keeping the default and letting operators tune per-provider via the new fields.

  2. New StreamFirstChunkTimeout / StreamIdleTimeout on BuildParams have no callers — no config/flag sets them. Wire them up in this PR or drop until wired.

Nit: internal/core/agent.go:119 still says default (60s) — stale after this change.

@hchenxa
hchenxa force-pushed the timeoutissue branch 2 times, most recently from 2295b19 to 2dd4f4c Compare July 13, 2026 02:56
@hchenxa

hchenxa commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

@yanmxa thanks for your comments, it make sense, i'd like to revert the default value changes and will introduce a new setting for streaming timeout

@hchenxa

hchenxa commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

@yanmxa please take a look at again

Signed-off-by: hchenxa <hchenxa1986@qq.com>
@yanmxa
yanmxa merged commit 66102d0 into genai-io:main Jul 13, 2026
3 checks passed
@yanmxa yanmxa mentioned this pull request Jul 13, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants