From 25759aecd7a637859a99a9e750e9e95997e32b9d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 17:33:44 +0000 Subject: [PATCH] Post-review follow-ups from the release-hardening pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - resumeRotation: an unresolvable current profile ('' selected, or the manually picked profile deleted) now counts as divergence — there is no profile the slot could legitimately be on, so the next generation re-picks and switches instead of draining the stale counter unswitched. - tryAdvanceFromFailure takes a notify option: startRotation and skipCurrentSlot pass notify:false since their callers already toast the returned error — one failed click no longer stacks two error toasts. The mid-rotation path, which has no caller to report to, keeps its toast. - numOr() treats null/undefined/'' as missing rather than coercing to 0, so count/weight fields imported as null prefill their real defaults (3/2/5/1) instead of a below-min 0; clearing an input now stores the fallback too. - Rotation tab mirrors the bar's fix: the status dot hashes only a real profile name, never the 'unassigned slot' placeholder. - Scoped the regenerate acceptance criterion to solo chats: group-chat regenerates are 'normal' end to end at the event level and carry no signal to filter on; documented as a known limitation. - TESTING.md criterion 13 describes what bind/unbind actually show (toast + Rotation-tab select) — a bare command's return value is never displayed. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01XDAj3aSdkvffKR6ZetqcyV --- CLAUDE.md | 4 ++-- TESTING.md | 6 +++--- src/events.js | 28 ++++++++++++++++++++-------- src/ui/queueEditor.js | 4 ++++ src/ui/tabs/rotation.js | 4 +++- 5 files changed, 32 insertions(+), 14 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 398d8bb..02d222c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -50,7 +50,7 @@ The extension is a thin scheduler layered on top of SillyTavern's existing Conne ### Hook points (SillyTavern event system) Use `eventSource.on(event_types.X, handler)` for all of these. Import `eventSource` and `event_types` from `../../../../script.js` (verified — see "Verified ST internals" below). -- `MESSAGE_RECEIVED` — primary trigger for advancing the response counter. **Emitted with `(messageId, type)`** where `type` is one of `'swipe'`, `'continue'`, `'append'`, `'appendFinal'`, `'regenerate'`, `'impersonate'`, `'quiet'`, `'first_message'`, `'command'` (/sendas-inserted character messages), `'extension'` (extension-inserted messages, e.g. stable-diffusion), `'normal'`, or `undefined` for a normal generation. **Advance the counter only when `type` is undefined/null/`'normal'`** — `isCountableGeneration()` is a strict whitelist so unknown future types fail closed. One trap the emitted type alone cannot catch: a **non-streaming regenerate** deletes the AI message being redone *before* generating, so by the time `saveReply` emits `MESSAGE_RECEIVED` the preceding message is the user's and the type is **coerced to `'normal'`** (streaming regens emit `'regenerate'` faithfully). `events.js` therefore also records the type each non-dry `GENERATION_STARTED` carried (`lastGenerationType`) and refuses to count a message whose *generation* started non-countable. +- `MESSAGE_RECEIVED` — primary trigger for advancing the response counter. **Emitted with `(messageId, type)`** where `type` is one of `'swipe'`, `'continue'`, `'append'`, `'appendFinal'`, `'regenerate'`, `'impersonate'`, `'quiet'`, `'first_message'`, `'command'` (/sendas-inserted character messages), `'extension'` (extension-inserted messages, e.g. stable-diffusion), `'normal'`, or `undefined` for a normal generation. **Advance the counter only when `type` is undefined/null/`'normal'`** — `isCountableGeneration()` is a strict whitelist so unknown future types fail closed. One trap the emitted type alone cannot catch: a **non-streaming regenerate** deletes the AI message being redone *before* generating, so by the time `saveReply` emits `MESSAGE_RECEIVED` the preceding message is the user's and the type is **coerced to `'normal'`** (streaming regens emit `'regenerate'` faithfully). `events.js` therefore also records the type each non-dry `GENERATION_STARTED` carried (`lastGenerationType`) and refuses to count a message whose *generation* started non-countable. (Group-chat regenerates are `'normal'` at both events and remain uncatchable — see acceptance criterion 6.) - `GENERATION_STARTED` — emitted with `(type, options, dryRun)`. The moment to fire the profile switch *before* the next generation, if the rotation says it's time to switch. **Skip when `dryRun === true`** (ST emits this for prompt-token-counting and similar dry runs) and skip non-normal `type` values (same set as above). This is critical: we switch profiles *before* the model generates, not after. - `MESSAGE_SWIPED` — fires when the user navigates between **existing** swipes via the left/right arrows; not on swipe-regeneration. Largely irrelevant to the scheduler since the `type` filter on `MESSAGE_RECEIVED`/`GENERATION_STARTED` already handles regen-swipes. - `CHAT_CHANGED` — emitted as `'chat_id_changed'`. Reload per-chat rotation state when the user switches chats. @@ -527,7 +527,7 @@ The extension is "done" when all of the following are true on a fresh ST install 3. A user with at least 2 connection profiles can create a queue, save it, and activate it on a chat. 4. In **sequential** mode with fixed counts (e.g. A=3, B=2, C=4), generating 9 messages causes the active profile to be A for the first 3, B for the next 2, C for the next 4. Verified by checking the connection profile selector value before each generation. 5. Swiping a message (regenerate-as-swipe) does not advance the counter — verified via the countable-type whitelist on `MESSAGE_RECEIVED`. -6. Regenerating a message does not advance the counter, **with streaming on or off** — streaming regens carry `type === 'regenerate'`; non-streaming regens arrive coerced to `'normal'` and are caught by the `lastGenerationType` guard (see Hook points). +6. Regenerating a message does not advance the counter, **with streaming on or off** (solo chats) — streaming regens carry `type === 'regenerate'`; non-streaming regens arrive coerced to `'normal'` and are caught by the `lastGenerationType` guard (see Hook points). Known limitation: **group-chat** regenerates are `'normal'` end to end (regenerateGroup deletes the reply, then generates normally) and carry no event-level signal to filter on, so they still count there. 7. In **weighted-random** mode with weights 1/1/1 and run length 1, profiles switch every message and over 100 messages each profile is used roughly evenly (within reasonable variance). 8. With `noRepeatInRow: true`, no two consecutive responses ever come from the same profile (verified over 50+ messages). 9. Switching chats preserves each chat's independent rotation state. diff --git a/TESTING.md b/TESTING.md index 4985a88..8fbd7cf 100644 --- a/TESTING.md +++ b/TESTING.md @@ -111,12 +111,12 @@ Subjective: with rotation active and counter `> 1`, send a message; the number i In the chat input, type each command: -- `/roulette-status` → echoes `Roulette: off` or `Roulette: · · N left`. +- `/roulette-status` → posts a system message in the chat: `Roulette: off` or `Roulette: · · N left`. (A bare command's *return value* is never displayed by ST — only the system message is visible.) - `/roulette-start seq-test` → activates `seq-test`. The bar lights up. - `/roulette-skip` → forces an immediate slot advance. The lit dot moves; counter resets to the new slot's count. - `/roulette-stop` → deactivates. The bar returns to hollow idle dots. -- `/roulette-bind seq-test` → binds the current character. Echoes `seq-test`. -- `/roulette-unbind` → removes it. Echoes the queue name that was unbound. +- `/roulette-bind seq-test` → binds the current character. No chat output of its own; with no rotation running you get the auto-start toast (`Roulette: started "seq-test" for `), and the Rotation tab's Auto-start select now shows `seq-test`. +- `/roulette-unbind` → removes the binding. No chat output; verify via the Rotation tab's Auto-start select reading `Nothing`. **Pass:** All six commands behave as described, with no console errors. diff --git a/src/events.js b/src/events.js index c5c23d4..5fc9da1 100644 --- a/src/events.js +++ b/src/events.js @@ -50,6 +50,10 @@ let switchInFlight = null; // promise guard — only one switch at a time * that message still carried 'regenerate', so we remember it and consult it * when counting. (Assumes generations don't interleave — ST serializes the * main generation pipeline.) + * + * Known hole: GROUP-chat regenerates are indistinguishable at the event + * level — regenerateGroup() deletes the old reply and re-generates typed + * 'normal' end to end, so they still count. Solo chats are fully covered. */ let lastGenerationType; @@ -119,7 +123,7 @@ export async function startRotation(queueId) { // Mark this slot as failed and try to advance immediately. failedSlotIndices.add(pick.slotIndex); consecutiveFailures++; - const recovered = await tryAdvanceFromFailure(queue, pick.slotIndex); + const recovered = await tryAdvanceFromFailure(queue, pick.slotIndex, { notify: false }); if (!recovered) { stopRotation(); return { ok: false, error: `Failed to switch to "${slot.profileName}" and could not recover.` }; @@ -165,7 +169,10 @@ export function resumeRotation() { const queue = state.activeQueueId ? findQueue(state.activeQueueId) : null; const slot = queue?.slots.find(s => s.id === state.currentSlotId); const loadedProfile = currentProfileName(); - const diverged = !!(slot && loadedProfile && slot.profileName !== loadedProfile); + // An unresolvable current profile ('' selected, or the manually + // picked profile was deleted) is divergence too — there is no profile the + // slot could legitimately be "on", so the next generation must re-switch. + const diverged = !!(slot && slot.profileName !== loadedProfile); updateChatState(s => { s.manuallyOverridden = false; if (diverged) { @@ -212,7 +219,7 @@ export async function skipCurrentSlot() { // try to recover failedSlotIndices.add(next.slotIndex); consecutiveFailures++; - const recovered = await tryAdvanceFromFailure(queue, next.slotIndex); + const recovered = await tryAdvanceFromFailure(queue, next.slotIndex, { notify: false }); if (!recovered) { stopRotation(); return { ok: false, error: `Skip failed and recovery exhausted.` }; @@ -230,33 +237,38 @@ export async function skipCurrentSlot() { * Attempt to advance past a failed slot. Recurses (bounded by * MAX_CONSECUTIVE_FAILURES) until a switch succeeds or we give up. * + * `notify` toasts the give-up reason. Pass false from paths whose caller + * already surfaces the returned error (startRotation/skipCurrentSlot) — + * otherwise one failed click stacks two error toasts. The mid-rotation + * GENERATION_STARTED path has no caller to report to, so it keeps the toast. + * * @returns {Promise} true if recovery succeeded */ -async function tryAdvanceFromFailure(queue, lastFailedIndex) { +async function tryAdvanceFromFailure(queue, lastFailedIndex, { notify = true } = {}) { if (consecutiveFailures >= MAX_CONSECUTIVE_FAILURES) { const msg = `Roulette: ${consecutiveFailures} consecutive profile-switch failures. Halting rotation.`; console.error(msg); - if (typeof toastr !== 'undefined') toastr.error(msg); + if (notify && typeof toastr !== 'undefined') toastr.error(msg); return false; } const next = advanceSlot(queue, lastFailedIndex, [...failedSlotIndices]); if (!next) { const msg = 'Roulette: no eligible slot remains. Halting rotation.'; console.error(msg); - if (typeof toastr !== 'undefined') toastr.error(msg); + if (notify && typeof toastr !== 'undefined') toastr.error(msg); return false; } const slot = queue.slots[next.slotIndex]; if (!profileExists(slot.profileName)) { failedSlotIndices.add(next.slotIndex); consecutiveFailures++; - return tryAdvanceFromFailure(queue, next.slotIndex); + return tryAdvanceFromFailure(queue, next.slotIndex, { notify }); } const ok = await switchProfile(slot.profileName); if (!ok) { failedSlotIndices.add(next.slotIndex); consecutiveFailures++; - return tryAdvanceFromFailure(queue, next.slotIndex); + return tryAdvanceFromFailure(queue, next.slotIndex, { notify }); } await applyTuningOnSwitch(slot, queue); updateChatState(s => { diff --git a/src/ui/queueEditor.js b/src/ui/queueEditor.js index 0156a0a..fc7f133 100644 --- a/src/ui/queueEditor.js +++ b/src/ui/queueEditor.js @@ -264,6 +264,10 @@ function readWeightedRunCount(modeSel, fixed, min, max) { } function numOr(v, d) { + // Nullish/empty means "missing", not zero — Number(null) and Number('') + // are both 0, which would render a below-min 0 in inputs whose real + // default is 3/2/5/1 (e.g. a count field imported as null). + if (v === null || v === undefined || v === '') return d; const n = Number(v); return Number.isFinite(n) ? n : d; } diff --git a/src/ui/tabs/rotation.js b/src/ui/tabs/rotation.js index be39f88..e417e55 100644 --- a/src/ui/tabs/rotation.js +++ b/src/ui/tabs/rotation.js @@ -147,7 +147,9 @@ function refreshStatus(container, activeQueue, shownQueue, state, running, pause const slot = activeQueue.slots.find(s => s.id === state.currentSlotId); const profile = slot?.profileName || 'unassigned slot'; profileEl.textContent = profile; - profileEl.style.setProperty('--dot-color', colorForProfile(profile) ?? 'var(--roulette-text-muted)'); + // Hash the REAL profile name only — hashing the placeholder string + // would light the dot in a vivid colour that reads as a profile. + profileEl.style.setProperty('--dot-color', colorForProfile(slot?.profileName) ?? 'var(--roulette-text-muted)'); profileEl.classList.remove('roulette-rot-profile-idle'); metaEl.textContent = paused ? `${activeQueue.name} · paused by a manual profile switch`