Skip to content

[replay 7304] feat(illustrator): Run Interval per chat - #472

Open
Promansis wants to merge 1 commit into
replay-base/7304from
replay/7304-gfb2
Open

Promansis wants to merge 1 commit into
replay-base/7304from
replay/7304-gfb2

Conversation

@Promansis

Copy link
Copy Markdown
Owner

Dottore replay shadow (fork only) of upstream PR 7304 at d9cda66, the first head CodeRabbit reviewed.

Original title: feat(illustrator): Run Interval per chat

The Illustrator's Run Interval now belongs to each chat, so one chat can
draw every message, another every 5 and another only when asked (0).

- New chat metadata field illustratorRunInterval (0-100, null = agent
  default), clamped by the chat metadata route.
- resolveEffectiveAgentSettings applies the chat value through
  applyIllustratorChatRunInterval, so automatic cadence (and the cadence
  status route) uses it; 0 still means manual-only.
- Chat Settings Illustrator card (Roleplay and Game agent cards) gets a
  Run Interval field with the usual chat override / Use agent default.
- The add-agent window (drawer and setup wizard) shows the chat value,
  saves it to the chat and no longer writes runInterval into the global
  Illustrator config; other agents are unchanged.
- Agent Editor help says its value is the default for chats.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

🎭 Dottore Review — Concluded

Tip

Review posted. The specimen has left the observation table.

  • Commit: d9cda66 - feat(illustrator): set Run Interval per chat

@github-actions

Copy link
Copy Markdown

🎭 Dottore Review

Dottore Verdict: Repair Required

Warning

REPAIR REQUIRED
Actionable findings or blocking proof gaps remain for this head.

Findings Nitpicks Controls Reviewed Head Mode
3 0 1 warn d9cda661 full

Note

Mode: full
Head: d9cda66 - feat(illustrator): set Run Interval per chat
Base: e0dd772 - Merge pull request Pasta-Devs#7297 from Pasta-Devs/fix/chat-chrome-text-html-7296

🧭 Specimen Summary

  • The specimen moves Illustrator cadence into chat metadata, applies the override during agent resolution, and exposes editing and reset controls alongside cadence reporting. The main mechanism respects zero and leaves other agents untouched, but default parsing, reactivation, and asynchronous resets reveal three less disciplined paths.

🔎 Isolated Defects

Severity Location Finding
⚠️ MEDIUM packages/client/src/components/chat/ChatSettingsDrawer.tsx:2015 (RIGHT) Inherited string intervals disagree with execution
⚠️ MEDIUM packages/client/src/components/chat/AgentAddSetupFields.tsx:474 (RIGHT) Preserve explicit cadence when re-adding Illustrator
⚠️ MEDIUM packages/client/src/components/chat/ChatSettingsDrawer.tsx:2263 (RIGHT) A pending interval save can overtake the reset

🧹 Minor Imperfections

  • None recorded.
🤖 Copy prompt for isolated Dottore findings
Task: Fix `packages/client/src/components/chat/ChatSettingsDrawer.tsx:2015` on the RIGHT side.
Finding: Inherited string intervals disagree with execution
Severity: medium
Goal: Without a chat override, the displayed Illustrator interval equals the effective interval used by execution and the cadence endpoint.
Cover: Resetting to the agent default and reopening the card must preserve that agreement.; Both add-agent previews must interpret inherited intervals consistently.
Avoid: Treating every invalid or negative inherited value as zero would introduce manual-only behavior that execution does not share.; Null chat overrides must remain inheritance rather than numeric zero.
Acceptable fixes: Use a shared pure resolver matching runtime interval semantics for inherited values, while retaining separate chat-override normalization.
Proof required: Cover inherited numeric 0, string "0", positive numeric strings, missing values and invalid values; compare card, cadence and execution results.; Verify an unchanged correctly displayed inherited value creates no chat override.
Run the narrowest relevant check. If stale, leave code unchanged and record why.

Task: Fix `packages/client/src/components/chat/AgentAddSetupFields.tsx:474` on the RIGHT side.
Finding: Preserve explicit cadence when re-adding Illustrator
Severity: medium
Goal: An explicit chat interval survives removal and reactivation until an intentional interval edit or reset changes it, including when it equals the agent default.
Cover: Both the setup wizard and Chat Settings add flow use this helper and must preserve matching explicit intervals.; Missing or null intervals must continue inheriting the agent default when reactivation makes no interval change.
Avoid: Always persisting the displayed value would silently pin chats that currently inherit the default.; Truthiness checks would discard the valid manual-only value 0.
Acceptable fixes: Preserve a valid existing override when the selected value matches the default, while retaining inheritance for an unchanged missing/null value; keep explicit reset available.
Proof required: Exercise chat override 0, agent default changed to 0, removal and re-addition, then agent default changed to 5; the chat must retain 0 and skip automatic Illustrator runs.; Cover matching positive overrides and unchanged inherited values through both add interfaces.
Run the narrowest relevant check. If stale, leave code unchanged and record why.

Task: Fix `packages/client/src/components/chat/ChatSettingsDrawer.tsx:2263` on the RIGHT side.
Finding: A pending interval save can overtake the reset
Severity: medium
Goal: Successful interval commits and resets persist in interaction order, with the final successful choice governing execution.
Cover: Rapid interval edits, reset-after-edit and edit-after-reset must share ordering.; A rejected earlier save must not prevent a later reset from completing.
Avoid: Client cache-version guards cannot order server writes.; Serializing storage operations by arrival order cannot recover the user's interaction order.; Queuing only reset calls leaves interval commits free to overtake them.
Acceptable fixes: Enable the existing shared metadata save queue for both interval writes and resets, with failure reconciliation.
Proof required: Delay a positive interval request, issue reset with agent default 0, then release the request; persisted metadata must retain inheritance and cadence must remain manual-only.; Exercise successive edits and an earlier rejected save followed by a successful reset.
Run the narrowest relevant check. If stale, leave code unchanged and record why.

✅ Control Checks

Status Type Check Detail
⚠️ WARN Review Limitation Verification Coverage 2 candidate(s) went unexamined (0 past the 20-claim cap, the time limit or without a verdict, 2 after a checker error), so they were withheld.

🧪 Observations

  • Finder: Traced missing, null, zero, numeric-string, fractional, and out-of-range chat values through normalization, metadata patching, effective settings, and the automatic cadence gate.
  • Finder: Compared inherited agent defaults across the new card, both add previews, the cadence endpoint, and shouldSkipAgentByMessageInterval.
  • Finder: Followed remove/re-add state through the wizard's activeAgentIds-only removal and the metadata builder's equality-triggered null reset.

📝 Lab Notes

  • Dottore examined 11 candidate(s) against the code: 3 confirmed, 6 rejected, 0 unresolved, 2 unexamined.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Dottore Review — isolated defects from the specimen.

const illustratorChatRunInterval = normalizeIllustratorRunInterval(metadata.illustratorRunInterval);
const illustratorRunInterval =
illustratorChatRunInterval ??
normalizePositiveInteger(illustratorDefaults.runInterval, 5, MAX_ILLUSTRATOR_RUN_INTERVAL, 0);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ MEDIUM: Inherited string intervals disagree with execution

Location: packages/client/src/components/chat/ChatSettingsDrawer.tsx:2015 (RIGHT)

The card invents a default: with no chat override, persisted runInterval: "0" survives the settings merge but the numeric-only normalizer displays 5, while execution treats it as manual-only. Positive numeric strings other than "5" produce the same display mismatch. Committing the apparent 5 cannot repair the discrepancy, because the unchanged-value guard suppresses that write.

Tip

Suggested fix: Give the display the runtime's interpretation of inherited intervals, including supported numeric strings; a default should describe execution, not contradict it.

);
applyIllustratorDefault("illustratorUseAvatarReferences", setup.useAvatarReferences, defaults?.useAvatarReferences);
if (options?.runInterval != null) {
applyIllustratorDefault("illustratorRunInterval", options.runInterval, defaults?.runInterval);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ MEDIUM: Preserve explicit cadence when re-adding Illustrator

Location: packages/client/src/components/chat/AgentAddSetupFields.tsx:474 (RIGHT)

The add flow mistakes equality with the agent default for permission to erase a chat override. With an explicit chat interval of 0 and an agent default subsequently changed to 0, removing and re-adding Illustrator prefills 0, then writes null because the values match. A later agent-default change to 5 therefore restores automatic images in a chat whose manual-only choice was never reset.

Tip

Suggested fix: Preserve an existing explicit interval through reactivation, even when it matches the agent default; let the explicit reset control restore inheritance.

</label>
<AgentDefaultStatus
overridden={illustratorChatRunInterval !== null}
onReset={() => updateMeta.mutate({ id: chat.id, illustratorRunInterval: null })}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ MEDIUM: A pending interval save can overtake the reset

Location: packages/client/src/components/chat/ChatSettingsDrawer.tsx:2263 (RIGHT)

The reset dispatches null independently of the preceding interval save; the metadata hook's ordering queue is opt-in and this drawer does not enable it. If the positive-interval request reaches storage after the reset, that older choice becomes the persisted override despite the newer interaction. With an agent default of 0, the supposed return to manual-only behavior instead restores automatic generation.

Tip

Suggested fix: Route interval commits and resets through the same per-chat save queue, so transport timing cannot choose the final cadence.

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