Skip to content

[Lint 04] fix: burn down frontend eslint errors; make CI lint blocking - #235

Merged
willchen96 merged 2 commits into
Open-Legal-Products:mainfrom
amal66:olp-pr/lint-burndown
Jul 22, 2026
Merged

[Lint 04] fix: burn down frontend eslint errors; make CI lint blocking#235
willchen96 merged 2 commits into
Open-Legal-Products:mainfrom
amal66:olp-pr/lint-burndown

Conversation

@amal66

@amal66 amal66 commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to #232: burns the frontend eslint error backlog down from 23 errors / 40 warnings to 0 errors / 40 warnings, then flips the CI lint step from advisory (continue-on-error: true) to a blocking gate. All edits are minimal and behavior-preserving — no rule is disabled repo-wide.

Stacking note: this branch stacks on #232 and includes its ci.yml commit — please review the top commit only (fix: burn down frontend eslint errors; make CI lint blocking). Will be Graphite-stacked properly once write access allows.

Changes

Rule Count How it was resolved
@typescript-eslint/no-explicit-any 5 Fixed. ChatView.tsx used (msg as any).files / .workflow / .error, but the Message interface (shared/types.ts) already declares all three fields with exactly the shapes the UserMessage / AssistantMessage props expect. Casts removed; plain property access type-checks cleanly (verified by next build, which runs tsc).
react/no-unescaped-entities 1 Fixed. support/page.tsx: We'llWe'll.
react-hooks/set-state-in-effect 17 Targeted // eslint-disable-next-line with a one-line reason at each site (details below). Every occurrence is an intentional effect-driven state pattern; any "real" fix (derive-during-render, key-based remount, useSyncExternalStore) would change runtime behavior, so per-site disables were the safe call.

The 17 set-state-in-effect disables and their reasons

Site Reason
modals/Modal.tsx:64 SSR portal gate — hasMounted must flip after first client mount
hooks/useSelectedModel.ts:19 Hydration-safe localStorage read; SSR must render the default model
hooks/useFetchDocxBytes.ts:52 Clear stale bytes when documentId is removed, inside the fetch effect
contexts/ChatHistoryContext.tsx:76 Clear chat state on logout inside the effect that loads chats
assistant/CaseLawPanel.tsx:219 Sync path of an async fetch effect: serve prop/cache data without a loading flash
assistant/CaseLawPanel.tsx:272 Reset active opinion after opinions load
assistant/CaseLawPanel.tsx:276 Sync quote list when the tab prop changes
assistant/CaseLawPanel.tsx:324 Reset quote selection when the quote set changes
assistant/ChatView.tsx:86 Reset per-chat UI state when switching chats
assistant/ChatView.tsx:522 Initial scroll-button state must be measured from the live DOM
assistant/ChatView.tsx:556 Hide messages until scroll position is restored to avoid a visible jump
assistant/CitationQuotesHeader.tsx:50 Collapse list view when quotes drop to a single item
assistant/PreResponseWrapper.tsx:32 Streaming open/minimize latch (existing comment documents why)
assistant/AskInputPopup.tsx:245 Auto-submit when every question is answered; submit() sets state as part of the side effect
shared/MfaLoginGate.tsx:24 Sync fast paths of the async MFA check effect
shared/MfaLoginGate.tsx:67 Clear gate when a recent MFA verification exists instead of redirecting
tabular/TRChatPanel.tsx:167 Timed "Done" flash on the active→idle transition

CI

.github/workflows/ci.yml: removed continue-on-error: true from the frontend lint step and rewrote the step comment (and the header note) — lint is now a blocking gate since the error backlog is at zero. Warnings do not fail the step.

Why

#232 landed the lint step as advisory-only because main carried 23 inherited eslint errors and a blocking gate would have turned every PR red on debt it didn't create. With the backlog at zero, the gate can hold the line: any new eslint error fails CI at the PR, instead of accreting silently.

Testing

  • cd frontend && npm run lint — exits 0: 0 errors, 40 warnings (was 23 errors / 40 warnings; warnings are pre-existing, mostly no-unused-vars on node ref params and no-explicit-any warnings in test-adjacent code).
  • npm test — no test script exists on this branch (predates the vitest harness PR); CI's npm test --if-present no-ops, as designed in [CI 03] ci: build and test workflow for backend and frontend #232.
  • NEXT_PUBLIC_SUPABASE_URL=https://placeholder.supabase.co NEXT_PUBLIC_SUPABASE_PUBLISHABLE_DEFAULT_KEY=sb_publishable_placeholder NEXT_PUBLIC_API_BASE_URL=http://localhost:3001 npm run build — passes (compiles + type-checks all routes), confirming the ChatView.tsx cast removals are type-safe.

🤖 Generated with Claude Code

QA Runner and others added 2 commits July 20, 2026 10:45
Ported from #46 onto current main, extended for this tree:
backend job runs npm ci, tests (--if-present, so it is safe to merge in any
order relative to the test-harness PR) and tsc build; frontend job runs
tests, eslint as an advisory step (main currently carries 23 lint errors —
flip to blocking once burned down), and a production next build with
placeholder NEXT_PUBLIC_* env (verified locally that the build succeeds and
contacts nothing); evals job runs node evals/run.mjs when present, else
skips.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Takes frontend/npm run lint from 23 errors / 40 warnings to 0 errors /
40 warnings, then removes continue-on-error from the CI lint step so it
gates merges.

Errors fixed outright:
- @typescript-eslint/no-explicit-any (5, ChatView.tsx): dropped
  redundant (msg as any) casts — Message already declares files,
  workflow, and error with the exact shapes the props expect.
- react/no-unescaped-entities (1, support/page.tsx): "We'll" ->
  "We&apos;ll".

Targeted disables (17, all react-hooks/set-state-in-effect): every site
is an intentional effect-driven state pattern — SSR/hydration mount
gates (Modal, useSelectedModel), reset-on-prop/identity-change
(CaseLawPanel x3, ChatView chat switch, CitationQuotesHeader,
ChatHistoryContext logout, useFetchDocxBytes), sync fast paths of async
fetch/check effects (CaseLawPanel, MfaLoginGate x2), DOM-measured state
(ChatView scroll button, message-visibility restore), and timed UI
latches (PreResponseWrapper, TRChatPanel, AskInputPopup auto-submit).
Rewriting any of them would change runtime behavior, so each carries a
// eslint-disable-next-line with a one-line reason instead of a fix or
a repo-wide rule change.

CI: lint step is now blocking; comments updated to say the backlog is
at zero.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


QA Runner seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@amal66 amal66 changed the title fix: burn down frontend eslint errors; make CI lint blocking [Test 04] fix: burn down frontend eslint errors; make CI lint blocking Jul 21, 2026
@amal66 amal66 changed the title [Test 04] fix: burn down frontend eslint errors; make CI lint blocking [Testing 04] [Test 04] fix: burn down frontend eslint errors; make CI lint blocking Jul 21, 2026
@amal66 amal66 changed the title [Testing 04] [Test 04] fix: burn down frontend eslint errors; make CI lint blocking [Testing 04] fix: burn down frontend eslint errors; make CI lint blocking Jul 21, 2026
@amal66 amal66 changed the title [Testing 04] fix: burn down frontend eslint errors; make CI lint blocking [Lint 04] fix: burn down frontend eslint errors; make CI lint blocking Jul 21, 2026
@willchen96
willchen96 merged commit 7d2ba4b into Open-Legal-Products:main Jul 22, 2026
1 check was pending
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.

3 participants