Skip to content

fix(chat): restore queue slots and allow 10 - #2236

Open
zerob13 wants to merge 1 commit into
devfrom
codex/fix-pending-input-limit-recovery
Open

fix(chat): restore queue slots and allow 10#2236
zerob13 wants to merge 1 commit into
devfrom
codex/fix-pending-input-limit-recovery

Conversation

@zerob13

@zerob13 zerob13 commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Allow up to 10 waiting messages per session and free a waiting slot as soon as a queued message is claimed. Previously, the UI hid the running message while the main process still counted it toward the five-message limit, so Enter and Queue could fail even with a visible count of 4/5.

The main admission limit, renderer submit gating, and waiting-lane counter/full hint now use 10. Claimed messages remain tracked for execution and recovery, but do not occupy waiting slots. A SQLite regression covers rejecting an eleventh waiting message, refilling after a claim, and refilling after deletion while the claimed message remains active.

BEFORE
+-------------------------------+
| Waiting 4/5                   |
| Four waiting messages         |
+-------------------------------+
| Draft            [Queue]      | --> Limit reached (one running)
+-------------------------------+

AFTER
+-------------------------------+
| Waiting 9/10                  |
| Nine waiting messages         |
+-------------------------------+
| Draft            [Queue]      | --> Waiting 10/10
+-------------------------------+

Validation:

  • pnpm run format:check, pnpm run i18n, pnpm run lint, and pnpm run typecheck.
  • 81 main-process tests across pending input storage, ownership, admission, and pumping.
  • 149 renderer tests across composer submission, pending input state, ChatPage, and PendingInputLane.
  • Queue-capacity regression passes with ELECTRON_RUN_AS_NODE=1 DEEPCHAT_REQUIRE_NATIVE_SQLITE=1 pnpm exec electron node_modules/vitest/vitest.mjs run --project main test/main/session/data/tables/deepchatPendingInputsTable.test.ts -t 'SessionPendingInputs queue capacity'.

The full native table suite has four unrelated existing failures in migration/transaction tests, reproduced with the unchanged test file. They involve mocked temporary-directory creation and obsolete direct table access on MainDatabase.

Summary by CodeRabbit

  • New Features

    • Increased the pending input queue capacity from 5 to 10 items per session.
    • Queue ordering is preserved as items are added and processed.
    • Claimed items remain available in the queue history while no longer counting toward the active limit.
  • Bug Fixes

    • Corrected active queue counts so claimed inputs are excluded from the pending capacity limit.
    • New inputs can be added after an existing item is claimed or deleted.

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 57b4ede8-3267-428f-8c3d-128a35dcbd1e

📥 Commits

Reviewing files that changed from the base of the PR and between f886d6e and 3a25083.

📒 Files selected for processing (5)
  • src/main/session/data/pendingInputStore.ts
  • src/main/session/data/pendingInputs.ts
  • src/renderer/src/components/chat/PendingInputLane.vue
  • src/renderer/src/stores/ui/pendingInput.ts
  • test/main/session/data/tables/deepchatPendingInputsTable.test.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The pending-input capacity increases from 5 to 10. Claimed inputs no longer count toward the active queue limit. Renderer limits and integration tests reflect the updated behavior.

Changes

Pending input capacity

Layer / File(s) Summary
Capacity enforcement and queue state
src/main/session/data/pendingInputs.ts, src/main/session/data/pendingInputStore.ts, test/main/session/data/tables/deepchatPendingInputsTable.test.ts
The backend limit increases to 10. Claimed inputs are excluded from active counts. The integration test covers capacity, recovery, ordering, and claimed state.
Capacity display alignment
src/renderer/src/stores/ui/pendingInput.ts, src/renderer/src/components/chat/PendingInputLane.vue
The renderer limit and pending-input lane default limit increase to 10.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 3a250

This change raises the waiting-message limit to ten and releases waiting capacity when messages are claimed while retaining them for execution and recovery. The updated backend, UI limits, and regression coverage align with that behavior, with no current merge-blocking risk identified.

Suggested reviewers: yyhhyyyyyy

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: restoring queue slots and increasing the limit to 10.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-pending-input-limit-recovery

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zerob13
zerob13 requested a review from yyhhyyyyyy September 5, 2026 15:19

@zerob13 zerob13 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Review: Approve ✅

(Comment review — GitHub does not allow approving one's own PR. Reviewed as if approving.)

Verified against f886d6e9b (merge-base with dev):

Core fix — correct. countActiveQueue now excludes claimed rows, which aligns the JS admission count with the pre-existing SQL semantics in countActiveBySession (AND NOT (mode = 'queue' AND state = 'claimed'), deepchatPendingInputs.ts:208) and with the renderer's activeCount (derived from listPendingInputs, which already filtered claimed). This resolves the UI-shows-4/5-but-Enter-fails mismatch described in the PR.

Claimed inputs remain tracked. Still returned by getInput/listActiveInputs (restart recovery), hasClaimedInput (drain gating), and getClaimedInput; release paths (releaseClaimedQueueInputTo, blockClaimedInput) correctly re-occupy a slot.

Races — none. ensureWithinLimit + insert run synchronously on the single-threaded main process; the async gap in sendQueuedMessage is re-checked authoritatively at the final synchronous admission point. Delete/move are transactional.

No stale assumptions of 5. Constants, error strings, i18n queueCount/limitReached (all 20 locales use {count}/{max}), and existing tests checked — no leftover 5 semantics. Off-by-one correct on all three gates (main throw, renderer isAtCapacity, lane hint).

Test — appropriate. The single new it covers the persistence contract (reject at 11, refill after claim with ordering, refill after delete while claimed stays active) via the public SessionPendingInputs API. Smallest durable coverage, nothing implementation-coupled.

Non-blocking notes (follow-up only)

  1. P3 — The limit now lives in three hand-synchronized places: MAX_ACTIVE_PENDING_INPUTS (pendingInputs.ts:12), MAX_PENDING_INPUTS (pendingInput.ts:6), and the prop default in PendingInputLane.vue:295. All three were updated consistently here, but a future limit change must again touch all three or the UI counter will disagree with the admission gate. Consider a shared constant in src/shared as a follow-up.
  2. P3countActiveQueue materializes rows and filters in JS while its sibling countActive delegates to a SQL COUNT. A SQL predicate would match the sibling's pattern and avoid row materialization on every enqueue. Not required in this PR given the smallest-change rule.

@yyhhyyyyyy yyhhyyyyyy 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.

Three inline notes. The first is a small regression, the other two are minor.

return this.database.deepchatPendingInputsTable
.listActiveBySession(sessionId)
.filter((row) => row.mode === 'queue').length
.filter((row) => row.mode === 'queue' && row.state !== 'claimed').length

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.

Excluding claimed here fixes the 4/5 mismatch, but the release paths (releaseClaimedQueueInputTo, releaseClaimedInput, restart recovery) move the item back to pending / retry_required with no capacity check. With 10 waiting + 1 claimed, a pre-user-fact rollback, release-for-mutation, or a restart release leaves 11 waiting items and the lane shows 11/10. Reproduced locally; before this change the same flow saturated at 5. Probably want admission to keep one slot reserved while an item is claimed, or cap the displayed count.

import type { SessionTranscript } from './transcript'

const MAX_ACTIVE_PENDING_INPUTS = 5
const MAX_ACTIVE_PENDING_INPUTS = 10

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.

Nit: the limit now lives in three places (this constant, renderer MAX_PENDING_INPUTS, and the activeLimit default in PendingInputLane.vue, which ChatPage never passes). A single shared constant would stop them drifting again.

})
})

describeIfNativeSqlite('SessionPendingInputs queue capacity', () => {

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.

FYI this suite is describe.skip under plain Node (17 skipped locally), and the test-main CI job runs plain Node, so this regression test won't execute in CI unless the file is added to the DEEPCHAT_REQUIRE_NATIVE_SQLITE=1 list.

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