Skip to content

Fix transient thread title inference failures - #1330

Merged
ymichael merged 5 commits into
mainfrom
bb/thr_hwqg5euaw9
Aug 11, 2026
Merged

Fix transient thread title inference failures#1330
ymichael merged 5 commits into
mainfrom
bb/thr_hwqg5euaw9

Conversation

@ymichael

@ymichael ymichael commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Summary\n\n- add BB_INFERENCE_FALLBACK as a live-reloadable bb-app config/environment setting, defaulting to codex/gpt-5.4-mini\n- use the configured primary model first and switch every thread-title path and commit-subject inference to the fallback only after timeouts, rate limits, or service-unavailable failures\n- apply the same two-attempt policy to asynchronous personal/reused-workspace titles without delaying thread startup\n- give Luna title inference a realistic 5-second attempt window and classify provider HTTP/stream failures as retryable\n- preserve structured Codex stream error codes even when a human-readable message is present\n- terminate and cancel SSE reads immediately when an error or response.failed event arrives instead of waiting for EOF\n- enforce one end-to-end daemon deadline across fetch and SSE reads, with host RPC delivery grace\n- bump the host-daemon protocol to 100 after rebasing over independent protocol changes on main\n\n## Validation\n\n- reproduced the missed personal-thread fallback from thr_fy866594ed: Luna returned codex_service_unavailable after 1.74s and the old async path made only one attempt\n- regression test drives that exact async lifecycle, verifies the fallback model is selected, applies the title after the thread becomes idle, and queues the provider rename\n- fresh personal-thread live probe on the restarted branch generated the title "Test title generation"\n- live configured fallback probe: Luna hit the forced transient timeout path; GPT-5.4 mini returned a structured title in 1.18s\n- affected Turbo typechecks pass for config, server, bb-app, templates, host daemon, host-daemon contract, and integration tests\n- full server suite passes: 160 files / 1,409 tests\n- full host-daemon suite passes: 45 files / 526 tests\n- 35-test host-daemon contract suite passes\n- generated guide templates are current and git diff --check passes

@SawyerHood

Copy link
Copy Markdown
Collaborator

🚨 SLOP COP 🚨 · review

I am SlopCop. I am reviewing this pull request for security, code quality, performance, architecture, and end-to-end behavior.

"Thread metadata inference timed out; retrying",
"Thread metadata inference failed transiently; retrying",
);
await delay(THREAD_METADATA_RETRY_DELAY_MS);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🚨 slopcop/review — The fixed delay can increase load during provider limits.

A 429 or 503 retry always waits 250 ms. The daemon does not pass Retry-After, so concurrent requests can retry too early.

Please preserve a bounded provider delay. Use a random delay when the provider gives no value.

Comment thread apps/host-daemon/src/codex-chatgpt-client.ts
Comment thread apps/host-daemon/src/codex-chatgpt-client.ts Outdated

@SawyerHood SawyerHood left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🚨 SLOP COP 🚨 · review

ELI5

The app asks a helper to name each new thread. This change gives the helper more time and another try.

It is like one extra turn to name a toy. Some failure notes still make the next try late or unsuccessful.

Findings

  • Medium: A server_error code can disappear when its event also has a message. A neutral message then skips the service retry.
  • Medium: The reader continues after response.failed. An open stream can consume the deadline and hide the retryable service error.
  • Medium: A 429 or 503 retry always waits 250 ms. This delay ignores Retry-After and can align concurrent retries.

I left inline comments for these findings.

Snapshot note

The requested SHA used protocol 98. Current main also used protocol 98, so that snapshot could not force a daemon update.

The PR moved to a10b91eb during this review. That head uses protocol 99 and fixes the protocol issue.

The other feature files have the same patch on both heads.

Architecture and security

The server owns retry policy. The daemon owns provider requests, deadlines, and error translation. This boundary is correct.

I found no harmful duplicate logic. Three response-fetch interfaces still contain an unused command field and should remove it.

I found no new authorization bypass, credential leak, or injection path.

Validation

  • git diff --check passed.
  • Turbo type checks passed for the daemon, server, and contract.
  • The contract passed 49 tests.
  • The daemon passed 524 tests.
  • The server passed 1,404 tests.
  • A browser test does not apply because this change has no UI route.

The GPT-5.6 review gate found the three medium issues above.

@ymichael
ymichael merged commit 05201ba into main Aug 11, 2026
10 checks passed
@ymichael
ymichael deleted the bb/thr_hwqg5euaw9 branch August 11, 2026 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants