fix(webui-v2): enforce TypeScript source conventions - #6057
Conversation
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughWebUI v2 adds a TypeScript AST-based source-convention checker, replaces ESLint linting, removes extensionful frontend imports, updates VM-based tests and JSX test harnesses, and documents the resulting lint and migration workflow. ChangesFrontend source conventions
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant pnpmLint
participant ConventionChecker
participant TypeScript
Developer->>pnpmLint: run lint
pnpmLint->>ConventionChecker: scan frontend/src
ConventionChecker-->>pnpmLint: report violations or pass
pnpmLint->>TypeScript: run typecheck
TypeScript-->>pnpmLint: report type errors or pass
pnpmLint-->>Developer: lint result
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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. Comment |
github-actions
Bot
added
scope: docs
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request completes the TypeScript migration for the WebUI v2 frontend and introduces a custom source convention checker to enforce extensionless relative imports, JSX usage, and TypeScript-only modules. Stale ESLint configurations and dependencies are removed, and existing tests are updated to comply with these rules. The review feedback recommends refactoring a highly coupled assertion in button.test.tsx to use the includesElementType helper, making the test more resilient to future component DOM changes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const children = rendered.props.children as ReactElement[]; | ||
| const content = children[1] as ReactElement<{ children: [unknown, unknown] }>; | ||
| assert.equal(content.props.children[0], false, "disabled without loading renders no spinner"); | ||
| assert.equal(content.props.children[1], "Save"); |
There was a problem hiding this comment.
The assertion here is highly coupled to the internal JSX structure of the Button component (e.g., expecting the children to be at index 1 and the conditional spinner to be at index 0 of its children). If the component's internal DOM structure is refactored (such as removing or adding wrapper elements), this test will break even if the behavior remains correct.
Using the existing includesElementType helper is much more robust and maintainable, as it recursively checks for the presence of the Spinner element without relying on exact array indices.
| const children = rendered.props.children as ReactElement[]; | |
| const content = children[1] as ReactElement<{ children: [unknown, unknown] }>; | |
| assert.equal(content.props.children[0], false, "disabled without loading renders no spinner"); | |
| assert.equal(content.props.children[1], "Save"); | |
| assert.equal(includesElementType(rendered.props.children, Spinner), false, "disabled without loading renders no spinner"); |
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.38% — 296980 / 347853 lines Per-crate breakdown (63 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
…#5850) Atomic roll-up of the 8-PR NEA-25 taxonomy stack onto current main. Extension is the only installable product object; tool/channel/auth are derived capability surfaces; runtime kind controls loading only; manifest projection (v2, host_api contracts) is the sole surface-discovery source of truth. The connectable-channels rail and the parallel `kind` taxonomy are removed and pinned by a zero-legacy gate. slack_bot and slack_personal are retired into one `slack` extension with bounded forward migrations. Extensions wire carries runtime + surfaces, not a conflated kind. Supersedes #5833, #5839, #5842, #5845, #5847, #5848, #5849, #5850. Conflicts with main since the train forked were reconciled preserving main's newer behavior (#5851 unified slack cleanup, #6054 get_conversation_info DM resolution, #5499 extension import, #6057 TS source conventions). provider_identity domain duplication removed; the residual is a legitimate up-layer port adapter. See the PR description for the per-PR crosswalk, resolutions, placement audit, and verification. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This branch was successfully deployed
1 participant
Summary
.js,.mjs, and.mtsmodules to.ts/.tsxhtml\...`` test helper with JSXpnpm linttogether with typecheckingRoot cause
The large frontend TypeScript/TSX conversion missed nine existing JavaScript-family files, and two more were added afterward. Vitest only discovers
.test.tsand.test.tsx, so the leftover.mjs/.mtssuites were silently skipped. The existing ESLint configuration also targeted only.js/.mjs, which meant it no longer inspected the TypeScript source while still returning success.Enforcement
The new convention gate scans authored modules under
frontend/srcand rejects:.ts/.tsx.js/.jsx/.mjs/.cjs/.ts/.tsx/.mts/.ctssuffixes on relative imports, re-exports, and dynamic importshtml\...`` tagged templatesNon-code assets, generated JavaScript bundle names, package imports such as
highlight.js, and explicit filenames passed to file APIs such asnew URL(...)remain allowed.Validation
pnpm --dir crates/ironclaw_webui_v2/frontend lintpnpm --dir crates/ironclaw_webui_v2/frontend test— 88 files, 708 testspnpm --dir crates/ironclaw_webui_v2/frontend buildcargo test -p ironclaw_webui_v2 --features webui-v2-beta— 170 testscargo clippy -p ironclaw_webui_v2 --all-features --tests -- -D warningsFocused red/green tests cover missing checker implementation, attributed dynamic imports, restored SSE dependencies, Button coverage migration, and JSX conversion.
Compatibility and rollback
This changes authored frontend source and development checks only. Browser bundle filenames, HTTP/static paths, persistence, configuration, and runtime schemas are unchanged. Rollback is a normal revert of this PR; there are no data migrations or hidden runtime side effects.