Skip to content

refactor(web): use the optional NumberField for per-source limits (UI 0.142.0) - #2600

Merged
0xPlayerOne merged 5 commits into
mainfrom
claude/ui-number-fields
Oct 18, 2026
Merged

0xPlayerOne merged 5 commits into
mainfrom
claude/ui-number-fields

Conversation

@0xPlayerOne

@0xPlayerOne 0xPlayerOne commented Oct 18, 2026 •

Copy link
Copy Markdown
Member

Summary

Moves the four per-source limits (document limit, content bytes, content characters, duration seconds) in SourceSettingsWorkflow.tsx from native number inputs to the shared optional NumberField (adea-ai/ui#444). A blank field is accepted and reported as null, a null limit shows blank, and only whole numbers inside the range reach the settings draft. The locked state still comes from sourceLocked(). The payload shape is unchanged: all four fields stay number | null, and the local optionalNumber helper is removed.

Part of #2568. Four commits:

  1. @adea-ai/ui 0.140.0 to 0.142.0, with package.json, apps/web/package.json, bun.lock and the NOTICE heading. 0.142.0 is the lowest version published to npm that contains chore(main): release 0.29.7 #444 (0.141.x exists as GitHub releases but was never published to npm). Its @adea-ai/themes range is ^0.10.4, which the pinned 0.10.6 satisfies, so no second themes copy is added; 0.142.1 and later require ^0.10.10. The two x-release-please-version markers in bun.lock were restored after the install.
  2. The NumberField adoption and a focused test.
  3. Regression tests for fractional, over-range and incomplete drafts and for the source-job lock, plus a note in docs/ingestion.md.
  4. (a48c91f, added after qualification testing) Keep the arrow and paging keys with the source setting fields: the accordion root cancelled ArrowUp, ArrowDown, Home, End, PageUp and PageDown from any field inside an open panel, so the limit fields could not be stepped with the arrow keys. This is also true on main today. The panel now stops those keys when the target is an input or textarea, and the accordion keeps its own trigger navigation. Commit and test: see Validation.

Source-card styling (#2586) stays deferred. This does not touch #2599 (the header change) and shares no files with it.

Validation

  • Commands run (Bun 1.4.3, base main 7d3d786), re-run at a48c91f: bun run lint, bun run format:check, bun run --cwd apps/web typecheck pass; bun run test passes (78 JavaScript test files); the web build passes within budgets (initial JS 517,660 / 525,000, complete JS 896,510 / 940,000, CSS 244,015 / 285,000). bun scripts/shared-notice-contract.test.mjs passed at the dependency commit.
  • New tests: blank stays null and only accepted numbers are reported; fractional (1.5), over-range and incomplete numeric drafts are not reported and the saved value is restored on blur; the four limits lock while a source job runs and unlock when it finishes; navigation keys in the limit fields are not cancelled (navigation keys in the source limit fields stay with the field, fails without the repair). The behaviour tests fail against the old native inputs.
  • NumberField verification: installed 0.142.0 matches release tag v0.142.0 source (sha256 of the file; lockfile integrity equals the registry integrity; the chore(main): release 0.29.7 #444 commit is in the tag).
  • Real browser (Chromium, built preview, ?demo):
    • Layout at 1280, 768, 390 and 320 px, 100% and 200% root text, focus and invalid value: all four fields are fully in view at 100% at every width; at 200% text at 390 and 320 px they sit in the settings panel's horizontally scrolling area, identically on main's native inputs. (Measured at the dependency head; the repair touches only key handling.)
    • Arrow keys at a48c91f: 24/24 checks (ArrowUp/ArrowDown step by 1 in all four fields, stop at min and max, an over-range draft is restored on Tab). The main baseline is 8/24.
    • Source-job lock at a48c91f: 9/9 checks with a scripted Tauri bridge so a real job starts, runs and finishes. Four fields lock while it runs, keep their values, take no focus or key, and unlock afterwards.
  • Evidence and a readable summary: cortana/pr2600-evidence/EVIDENCE.md in the project files.
  • CI or deployment links: normal checks run on this head; see the PR.

Impact and rollout

  • Security, privacy, compatibility, or performance impact is documented below
  • Rollback or follow-up work is documented below

Behavior difference: a typed value outside a field's range (for example 0 documents) was previously written into the draft as typed. Fractional drafts (for example 1.5) are now rejected as well as out-of-range ones: neither is reported, and the field is restored to the last accepted value on blur. Blank still saves null. This is documented in docs/ingestion.md. The complete JS graph grows by about 2.7 kB against main. In the source panel, Home, End and the arrow and page keys now act on the focused field instead of being cancelled.

Not covered: Firefox and WebKit, a real Tauri desktop window (the job lock was run through a scripted bridge), and the macOS titlebar inset (never tested).

Review notes

  • The pin is deliberately 0.142.0, not the newest 0.142.3, to avoid a second @adea-ai/themes copy. A later pin bump is a normal Renovate step.
  • 0.142.0 also ships the AI Elements foundation, which this app does not use.
  • Follow-up: the accordion in @adea-ai/ui should leave key events from inputs alone; this PR works around it in the one panel it affects, which does not change the shared package.

Related issue

Part of #2568 (stays open).


Generated by Claude Code

First published release containing the optional and disabled NumberField
(adea-ai/ui#444; 0.141.x was never published to npm). Its @adea-ai/themes range
(^0.10.4) is satisfied by the pinned 0.10.6, so no second themes copy is added.
NOTICE heading follows the pinned version and the release-please markers in
bun.lock are restored.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MfzvPs9KL9LLoLyPv5DiLP
The document, byte, character and duration limits were native number inputs
that parsed their own text. They now use NumberField with `optional` (adea-ai/ui#444):
a blank field is accepted and reported as null, a null limit shows blank, and only
values inside the range reach the settings draft. The locked state still comes from
`sourceLocked()`. Payload shape is unchanged: the four fields stay `number | null`.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MfzvPs9KL9LLoLyPv5DiLP

@0xPlayerOne 0xPlayerOne left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Review outcome: PENDING qualification for exact head 9c128bd. I personally reviewed the complete six-file patch and pinned NumberField implementation. The four ranges and null payloads are preserved; integer validation matches the backend limits, and manifest/lock versions agree. No concrete code blocker found so far. Before approval, complete the existing owner's narrow-layout, real keyboard/focus and running-source lock/unlock checks, including preserving a set limit during incomplete numeric input. Add focused fractional/upper-bound and dynamic disabled-state coverage where practical, and document that fractional drafts are now rejected as well as out-of-range drafts. Verify the installed package content corresponds to the reviewed NumberField implementation. Keep draft and auto-merge off pending these results; linked #2568 remains open.

…cument rejected drafts

Fractional and over-range drafts and incomplete numeric text are not reported
and the field restores the saved value on blur; the four limits lock while a
source job runs and unlock when it finishes. The ingestion doc now says
fractional drafts are rejected alongside range changes.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MfzvPs9KL9LLoLyPv5DiLP

@0xPlayerOne 0xPlayerOne left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Review outcome: PENDING small documentation correction and final qualification at 918f25f. I personally reviewed the complete initial patch and new tests/docs delta. The fractional/upper-bound/badInput and source-job lock/unlock cases address the main gaps; no production-code blocker found. Correct docs/ingestion.md: blank character limit uses connector defaults (not universally the global budget), and invalid drafts restore the latest accepted controlled value, which may be an unsaved edit, not necessarily the last saved value. For example saved7 -> accepted25 -> invalid2.5 -> blur restores25. Keep the candid limits: job-lock browser path was mock-bridge qualified, arrow keys were inconclusive on both baseline/candidate, and hosted draft-skipped checks are not passes. #2568 remains open. Re-review the final published head after docs correction.

…d draft restores

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MfzvPs9KL9LLoLyPv5DiLP

@0xPlayerOne 0xPlayerOne left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Coordinating personal review outcome: APPROVED for the complete seven-file scope at exact head bd4fccd. I reviewed the original migration/dependency patch, test additions and final documentation correction. Null preservation, integer/range rejection, incomplete-input retention and dynamic source-job lock/unlock are covered; pinned implementation/package matching was verified by the owner. Documentation now accurately distinguishes connector defaults and latest accepted unsaved values. No remaining code blocker found; advisory review agrees. This personal-review COMMENT does not replace required GitHub approval. Required hosted checks must run and pass after readiness; draft skips are not passes. Browser stepping remains inconclusive on both baseline and candidate; job locking uses mock-bridge integration, and no live-provider/native-inset qualification is claimed. #2568 stays open. Any head change invalidates this review.

@0xPlayerOne
0xPlayerOne marked this pull request as ready for review October 18, 2026 07:39
The accordion root cancels ArrowUp, ArrowDown, Home, End, PageUp and PageDown
to move between its triggers, even when they come from a field inside an
open panel. That stopped the four per-source limit fields stepping with the
arrow keys and moved no caret in the text fields. Stop those keys at the
panel when the target is an input or textarea, and keep the accordion's own
trigger navigation.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MfzvPs9KL9LLoLyPv5DiLP

Copy link
Copy Markdown
Member Author

Qualification at bd4fccde4a4553ccfee28131420617aa32e4e2b6 found one defect, now repaired and published at a48c91f24c414c3741da793258e236bb79a14f2c. Readiness and auto-merge are unchanged (ready for review, auto-merge OFF).

Defect. ArrowUp and ArrowDown did not step any of the four limit fields, and Home, End, PageUp and PageDown were swallowed too. The same happens on main today with native inputs. A bare number input and the field's own stepUp() both work. A delegated keydown handler on the accordion root cancels those keys for anything inside an open panel, so the fields never see them.

Repair (bounded, one component). AccordionContent in SourceSettingsWorkflow.tsx stops those six keys when the target is an input or textarea. The accordion keeps its own trigger navigation. The shared UI package is untouched.

Checks at a48c91f (Bun 1.4.3):

  • bun run lint, bun run format:check, apps/web typecheck: pass.
  • bun run test: 78 JavaScript test files pass.
  • Web build: passes within budgets.
  • New test navigation keys in the source limit fields stay with the field: passes with the repair and fails without it (cl-125 ArrowUp).
  • Real browser, arrow keys (arrow-keys.mjs): 24/24 after the repair; main baseline 8/24.
  • Real browser, source-job lock (source-job-lock.mjs, scripted Tauri bridge so a job really starts and finishes): 9/9. Four fields lock while it runs, keep their values, take no focus or key, and unlock afterwards with ArrowUp stepping again.

Evidence, scripts and screenshots are in cortana/pr2600-evidence/ (see EVIDENCE.md). Not covered: Firefox, WebKit, a real Tauri window, macOS titlebar inset. The previous approval was at the old head, so this head needs a fresh review. Full CI is running on the new head.


Generated by Claude Code

@0xPlayerOne 0xPlayerOne left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Personal review outcome: PENDING final owner evidence/CI at exact head a48c91f. I reviewed the complete two-file delta from previously source-approved bd4fccd. The panel-level bubbling key handler keeps six navigation keys on input/textarea targets without preventing their default behavior, while leaving accordion-trigger navigation intact; the regression explicitly checks both cases. No demonstrated source defect in this delta.

The updated PR reports 24/24 Chromium arrow-key checks and 9/9 source-job lock checks with a scripted bridge, plus applicable build/lint/types/tests. I am reconciling that receipt through the existing project lead/owner; it does not claim a real Tauri window or Firefox/WebKit coverage. The changed head needs fresh current CI/protection eligibility before merge; previous-head approval is not sufficient. #2568 remains partial and open. No release/deployment authorization.

@0xPlayerOne 0xPlayerOne left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Personal review outcome: SOURCE APPROVED at a48c91f. The existing project lead/owner confirmed this exact published head and qualification receipt (#2600 (comment)): 24/24 Chromium arrow checks, 9/9 scripted source-job lock/unlock checks, 78 local test files, lint/format/types/build passed. I personally reviewed the full two-file delta from the prior approved scope: input/textarea navigation keys stop bubbling at the source panel without preventDefault, and accordion triggers retain their navigation behavior. No remaining demonstrated source finding in this PR's scope. This narrowly scoped workaround does not claim a general shared Accordion fix. Real Tauri, Firefox/WebKit and titlebar behavior remain untested as documented. Applicable hosted CI and all protections must pass before merging; this own-author review comment is not a substitute for any required independent approval. #2568 remains incomplete/open.

@0xPlayerOne
0xPlayerOne merged commit 57c08c3 into main Oct 18, 2026
44 checks passed
@0xPlayerOne
0xPlayerOne deleted the claude/ui-number-fields branch October 18, 2026 22:14
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