Skip to content

fix(dashboard): stop QuotaCutoffModal drafts resetting on parent re-render (#7889) - #7921

Closed
diegosouzapw wants to merge 1 commit into
release/v3.8.49from
fix/7889-quota-cutoff-effect-deps
Closed

diegosouzapw wants to merge 1 commit into
release/v3.8.49from
fix/7889-quota-cutoff-effect-deps

Conversation

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Summary

Closes #7889

Provider Quota → "Edit cutoffs": the numeric cutoff input reset to the
provider default (or cleared) while the operator was still typing. Reported
against Codex but not specific to it — it hits any connection whose quota
data refreshes often enough during editing.

Root cause: QuotaCutoffModal's useEffect that seeds the local
drafts state was keyed on [isOpen, windows, current]. windows (array)
and current (object) are props whose identity changes on every parent
re-render — most directly, ProviderLimits/index.tsx rebuilds the windows
array with a fresh .map() on every render, and the same happens whenever
the parent's quota-polling/refresh state updates. React compares hook deps
by reference, so any unrelated parent re-render re-ran the effect mid-edit
and overwrote whatever the operator was typing with the last persisted
value.

Fix: reseed drafts only on the real "opened against a (possibly new)
connection" signal instead. QuotaCutoffModal now takes a connectionId
prop (the connection's stable id, already used elsewhere by the parent for
saveQuotaWindowThresholds/visibleQuotaData lookups) and the seeding
effect depends on [isOpen, connectionId]. windows/current are still
read for the actual seed values, but via refs kept fresh on every render —
so the effect always sees the latest data without re-running just because
those props got new array/object identities.

Root cause / fix

  • src/app/(dashboard)/dashboard/usage/components/ProviderLimits/QuotaCutoffModal.tsx
    — added connectionId prop, switched the seeding effect's dependency
    array from [isOpen, windows, current] to [isOpen, connectionId],
    reading windows/current through refs updated every render.
  • src/app/(dashboard)/dashboard/usage/components/ProviderLimits/index.tsx
    — threads connectionId={cutoffModalConn.id} into <QuotaCutoffModal>.

Test plan (TDD, Hard Rule #18)

  • Added tests/unit/ui/issue-7889-quota-cutoff-input-persists.test.tsx:
    • RED (confirmed against pre-fix code): typing "10" into the cutoff input,
      then re-rendering the parent with brand-new (but content-equivalent)
      windows/current references for the same connection, snapped the
      input back to the persisted value ("5") — reproducing the report exactly.
    • GREEN (post-fix): the same scenario now leaves "10" in the input.
    • Non-regression: closing/reopening the modal against a different
      connection (connectionId changes) still correctly reseeds the drafts
      to that connection's persisted values.
  • node scripts/check/check-file-size.mjs — OK, no ✗
  • node scripts/check/check-complexity.mjs — 2081 violations (baseline 2130), no regression
  • eslint --no-config-lookup --config eslint.sonarjs.config.mjs on changed files — clean
  • npm run typecheck:core — clean
  • eslint --suppressions-location config/quality/eslint-suppressions.json on changed files — exit 0
  • npm run test:vitest:ui — new test passes standalone and combined; 17 pre-existing failures in unrelated files (qdrant-config-card, runtime-page-client, etc.) reproduced only under full-suite load and pass cleanly in isolation — not touched by this change

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@diegosouzapw

Copy link
Copy Markdown
Collaborator Author

Thanks for the fast root-cause and fix — I verified it locally and it correctly reproduces and resolves #7889 (checked out the branch, ran the new tests/unit/ui/issue-7889-quota-cutoff-input-persists.test.tsx — both cases pass; lint/typecheck clean).

One thing worth flagging before merge: PR #7909 (@hydraxman) was opened ~55 minutes before this one and fixes the exact same bug in the exact same two files (QuotaCutoffModal.tsx + ProviderLimits/index.tsx), also with a working, tested, green solution (gating the reseed via wasOpenRef/seededConnectionIdRef instead of moving windows/current out of the deps array via refs). Both are technically sound — this one is slightly more idiomatic (no eslint-disable needed since the ref pattern is exempt from exhaustive-deps), but functionally equivalent.

Since #7909 is from an external contributor and was submitted first, I'd suggest merging that one and closing this one as subsumed, to keep the credit with the earlier external fix rather than my own duplicate. Happy to go the other way if there's a reason to prefer this implementation instead — let me know.

@diegosouzapw diegosouzapw added the duplicate This issue or pull request already exists label Jul 21, 2026
@diegosouzapw

Copy link
Copy Markdown
Collaborator Author

Closing this as subsumed by #7909 (just merged). @hydraxman opened an equivalent, also-tested fix for the exact same #7889 bug ~55 min earlier — same two files, both CI-green. Since this is my own duplicate of an earlier external contribution, the credit rightly goes to their PR (the project's whole focus is growing the contributor base over anything else). My ref-based approach worked equally well, but theirs landing first is the right outcome. Thanks anyway to everyone who looked at it.

@diegosouzapw
diegosouzapw deleted the fix/7889-quota-cutoff-effect-deps branch July 23, 2026 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

duplicate This issue or pull request already exists

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: 'Edit cutoffs' input resets to default / clears immediately on Provider Quota page

1 participant