refactor: close 2026-05 code audit (OCP closed + coverage gate)#51
Merged
Conversation
gandarfh
force-pushed
the
refactor/code-audit-cleanup
branch
2 times, most recently
from
7133c8d to
df19a5b
Compare
MarkdownEditor: a focus listener was attached inside a `useEffect` without cleanup. Moved to a ViewPlugin so teardown happens on view destroy. HttpBodyCM: the editor was being reconfigured on every commit, defeating the Compartment design. Restored the diff-only reconfigure pattern. NewConnectionModal: IPC errors from `create_connection_cmd` were swallowed silently. Now surfaced through the modal's error badge. Also fixes `scripts/coverage-check.sh`, which crashed on commits that only delete files (bash 3.2 empty-array + `set -u`). The empty check now runs before the array expansion.
Removes code with no reachable entry point: - ToolUseBlock + usePromptDialog — unreachable. - 7 dead feature islands (~10046 L across 82 files; the body is large because the islands cross-import each other): `src/components/ dev-tools/`, the legacy E2E block, the old TipTap-based vim implementation under `src/components/editor/vim/`, the unused `cm-scroll.browser.test`, the prior `<Box as="pre" dangerouslySetInnerHTML>` body renderer, the orphan promptDialog glue, and ~39 orphan test files surfaced by `tsc` + the suite sweep after each island was removed. - The dead E2E pipeline in `cm-block-widgets.tsx` (497 to 242 L), removed surgically — the surviving DiffViewer paths are preserved. - 8 unused npm dependencies (lockfile authoritative = repo root). - Documentation drift: a stale TipTap label + `CLAUDE.md` context and size claims that no longer matched the post-CM6 reality.
Four small extractions: - `lib/format/time.ts` (`formatElapsed` + `formatDurationCompact`), unifying 3 call sites. The 3 `relative-time` variants are left separate — their contracts diverge (HTTP vs DB vs history); noted in `time.ts`. - `blocks/execution-state.ts` — a canonical execution-state vocabulary (`idle | running | success | error | cancelled`). HTTP and DB fenced blocks re-export from it. The `ExecutableBlock.ts` flavor (with `cached`, no `cancelled`) stays separate — it belongs to the standalone/diff-viewer block, a different domain. - `jsString` and `pyString` consolidated into `backslashQuote` inside the codegen helpers. - `useConfigChangeRefresh` extracted from the 3 PageContainers, where the config-changed effect was duplicated verbatim.
- `stores/connections.ts` — a zustand store mirroring `environment.ts`. The `config-changed` watcher lives outside the store (a UI/effect concern, not state). - `ConnectionsPageContainer` + `ConnectionsList` migrated to the new store, eliminating 2 copies of `listConnections` + manual reload(). `pings` stays local (per-row, ephemeral). - `hooks/useCrossEnvVariables.ts` — a `Promise.all` fan-out for `listEnvVariables` per environment, cancellation-guarded. It reads the store but is intentionally not a zustand action: a single-shape store contract would diverge between the Variables and Environment pages. - The `ConnectionsPageContainer` prefetch effect is now keyed on the selected connection name (a memoized string) rather than the `connections` array, so unrelated refreshes (ping/CRUD/config) no longer re-dispatch `find_connection_uses_cmd`. - `useConfigSyncedResource(category, refresh)` composes `useConfigChangeRefresh` with the mount-refresh, collapsing the duplicated effect pair into one line per container. `EnvironmentsPageContainer` no longer needs a `useEffect`. A generic `useMasterDetail` was considered and rejected: only 2 sites diverge, thinly — the unifications above already collapsed the real duplication.
- `hooks/useInlineForm.ts` — minimal scope: one string field + touched + an attemptSubmit gate. Deliberately not a generic record with per-field errors/reset — that would exceed what the call sites need. - `NewVariablePopover` migrated to `useInlineForm` + `validateVariableName`, which also closes an ad-hoc-rule bug (dot/whitespace names are now properly rejected). - The 4 env/variable inline forms (NewVariable / NewEnvironment / Clone / Rename) migrated to `useInlineForm`. Behavior-preserving: error messages identical, 0 edits to the existing tests. - `ConnectionForm` rebuilt on `useReducer` + a pure module `connection-form-state.ts` (state / init / reducer / validateConnection / buildConnectionInput). Kills 18 useState calls and the props-to-state driver-to-port effect. Form complexity drops 41 to 18; the prop-effect and async-complexity lint warnings are eliminated. React Hook Form / zod were considered and rejected: the existing tests + the reducer + the pure validator cover the same surface with zero new dependencies.
The git store's 2s poll did `set({status: await gitStatus()})` /
`set({remotes: ...})` with a brand-new deserialized object every tick
even when git was byte-identical. The store reference changed every
2s, so every subscriber (the git pane-tab and the git side panel)
re-rendered on every no-op poll.
The fix is at the root cause: a structural-equality guard in
`refreshStatus` / `refreshRemotes` keeps the prior reference when the
polled payload is unchanged. Subscribers selecting `s.status` /
`s.remotes` then get a stable ref and don't re-render on a no-op poll.
A real git change still swaps the ref (subscribers update); a stale
`statusError` is still cleared without churning the status ref.
`React.memo` on the leaf panels was considered and deliberately not
added: their dominant re-render driver is non-ref-stable store data
(or, for the *Page components, their own data — they don't subscribe
to the git store at all). Wrapping them would be exactly the
noise-memoization this change avoids.
+4 store tests: ref identity across an unchanged poll, including a
subscriber-not-notified assertion; ref swap on a real change for both
status and remotes; stale-error clear without ref churn.
HttpFencedPanel was a 2634-line component holding toolbar, form/raw mode, result tabs, status bar, settings drawer, codegen, history, cache hydration, and the run state machine. Brought under the 600-line size gate by extracting cohesive units. View siblings (each under 600 L, extracted verbatim): - `HttpJsonVisualizer.tsx` — JSON tree + flattener for the visualize tab. - `HttpBodyView.tsx` — read-only CM6 body viewer + image/PDF/HTML preview overlay + pretty/raw/visualize toggle. - `HttpInlineEditors.tsx` — the 3 inline CM editors used by the form-table cells. - `HttpFormTables.tsx` — body-tab dispatcher + FormUrlEncodedTable + MultipartTable + BinaryFilePicker. Shared state machine: - `useExecutableBlock<T>` — the `idle -> running -> success | error | cancelled` machine + `AbortController` + collect-blocks/env + try/catch/finally. HTTP consumes it through an adapter (`validate`, `prepare`, `execute`, `elapsedOf`, `persist?`, `onOutcome?`, `onRunStart?`, `onProgress?`). DbFencedPanel deliberately does not consume it: its `runExplain` + `loadMore` co-mutate the same state as `run`, so migrating would force exposing raw setters and lose the encapsulation the hook exists to provide. Pure helpers: - `http-request-builder.ts` — `parseBody` / `deriveHost` / `httpElapsedOf` / `isValidHeaderName` / `HTTP_TOKEN_RE` / `buildExecutorParams`, with full module test coverage. Cohesive concerns split into 4 sibling hooks: - `useHttpRefsContext.ts` — refs context + stable getters. - `useHttpCacheHydrate.ts` — cache lookup on mount/body change; mutations skipped. - `useHttpCodegenSnippets.ts` — cURL/fetch/Python/HTTPie/.http pre-computation + `handleSendAs` + `copyAsCurl`. - `useHttpDrawerData.ts` — history + examples loaders + recordHistory + drawer actions. Result: HttpFencedPanel 2634 to 567 L. It now fits under the size gate with no escape-hatch pragma.
Before this change, adding a new block type meant editing `MarkdownEditor.tsx`, writing a CM6 extension, and adding a Portal mount component by hand. After it, adding a block type is: create `cm-<id>-block.tsx` + `<Id>FencedPanel.tsx` and append entries to `blockRegistry` + `blockPortals`. Zero edits to `MarkdownEditor.tsx`, `markdown-extensions.ts`, or `cm-slash-commands.ts`. - Generic CM6 fenced-block skeleton (`createFencedBlockExtension.tsx` + `widget-portal-registry.ts`): scanner, registry, decorations, keymap, StateField, and ref autocomplete factored out of cm-http-block and cm-db-block. `cm-http-block.tsx` 839 to 326 L, `cm-db-block.tsx` 1256 to 388 L; `cm-db-schema-complete.ts` (345 L) extracted as a sibling so cm-db-block fits the size gate. The 8 genuine HTTP/DB divergences (body-notify policy, dedupe rule, metaChanged fields, body decoration, form widget, error field, keymap semantics, closePanel height) are injected, not forced — documented in the generic modules' headers. - Generic `BlockWidgetPortals.tsx` (80 L) replaces the per-type `HttpWidgetPortals.tsx` (53 L) + `DbWidgetPortals.tsx` (56 L), deleted in the same commit. MarkdownEditor mounts it once per block type via the `blockPortals` array. - `BlockTypeSpec` (`block-registry.ts`, CM-level) + `BlockPortalEntry` (`block-portal-registry.tsx`, panel-level). The 3 composition sites (MarkdownEditor, markdown-extensions, cm-slash-commands) now consume the registry arrays. - `ExecutableBlockShell` renamed to `StandaloneBlockShell` to reflect its single consumer (StandaloneBlock); HTTP/DB panels never used it. Removed the dead `e2e` label from `BLOCK_LABELS`. - `invoke<T>` typed across the 47 call sites of `lib/tauri/commands.ts`. - Reachability re-sweep: 0 dangling imports; 2 stale doc comments (mirror-refs to the deleted portal files) fixed.
gandarfh
force-pushed
the
refactor/code-audit-cleanup
branch
from
df19a5b to
d22a040
Compare
The decomposition work above left HttpFencedPanel's new hooks and siblings untested, and several pre-existing files were already below the 80% threshold. This commit backfills tests until every file the branch touches passes the gate. Reached >=80% from below: - AboutSection 0% to pass - NewConnectionModalContainer 75% to 90% - active-editor 36% to pass - ConnectionForm 72% to 94% — interaction tests drive every per-field dispatch; non-Error throw branches via a raw-string throw in the mocked IPC handler - cm-block-widgets 36% to 96% — the DiffViewer widget machinery (findFencedBlocks + extractAlias + createBlockWidgetPlugin) - cm-db-block 41.6% to 97.8% — the SQL error-squiggle machinery (decorateDbBody classes; setDbBlockErrors to the error field to mark decorations; bodyLineColToOffset overflow + multi-line walk) - HttpFencedPanel 0% to 81.8% — mock-heavy orchestrator test; adapter callbacks captured via the useExecutableBlock mock - StandaloneBlock 69.9% to 98.9% - HttpBodyView 37.6% to 96.5% — pretty/raw/visualize toggle + the image/PDF/HTML preview overlay - HttpFormTables 36.4% to 98.1% — the FormUrlEncoded / Multipart / BinaryFile table editors - HttpInlineEditors 60% to pass - HttpJsonVisualizer 52.1% to pass - useHttpRefsContext / useHttpCacheHydrate / useHttpCodegenSnippets / useHttpDrawerData — all from untested to passing - widget-portal-registry 67.9% to pass - createFencedBlockExtension 73.7% to pass - block-registry / block-portal-registry / BlockWidgetPortals — all from untested to passing - shared / db-fenced-shared / cm-slash-commands — all to passing Two files carry a documented `// coverage:exclude file`: - `execution-state.ts` — a type-only module (compiles to empty .js, nothing for the coverage tool to instrument) - `EnvironmentsPageContainer.tsx` — the FLIP animation block needs `getBoundingClientRect` + `requestAnimationFrame`, which jsdom returns 0x0 for; visual coverage belongs to the Playwright project. Hygiene folded in: - `prettier --write` across the 51 touched files that had drifted from the project Prettier style (the CI `prettier --check` step enforces this). - Split `lib/tauri/commands.ts` into `commands.ts` (345 L) + `env-block-commands.ts` (284 L), re-exported. The `invoke<T>` typing pass + the Prettier reflow pushed `commands.ts` from 591 to 611 L, past the 600-line size gate; splitting pays the debt with no `size:exclude`, the same pattern the file already used for `block-history.ts`. - ESLint config tightened (max-params, max-lines-per-function, complexity — thresholds documented in-line). - Drop 8 unused npm dependencies (lockfile synced). - Fix 6 pre-existing TypeScript errors in test files. Final state: every touched file >=80% coverage, <=600 lines, tsc clean, prettier clean.
- Rewrites the block-model section: adding a new block type now means creating `cm-*-block.tsx` + a panel + one line in `blockRegistry` and `blockPortals` — no edits to MarkdownEditor, markdown-extensions, or cm-slash-commands. - `HttpFencedPanel.tsx`: drops the "monolith, pending split" label — it is 567 L now, decomposed into sibling hooks + view siblings. - `DbFencedPanel.tsx`: 779 L; notes why it keeps its in-place reducer rather than the shared state hook (runExplain/loadMore co-mutate state). - `BlockWidgetPortals.tsx` (generic) documented as the replacement for the per-type portal files. - `ExecutableBlockShell` to `StandaloneBlockShell` reference updated. - MarkdownEditor line count + the registry source-of-truth noted. No code changes — documentation only.
gandarfh
force-pushed
the
refactor/code-audit-cleanup
branch
from
d22a040 to
ea4a032
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resumo
Limpeza estrutural do frontend
httui-desktop— 65 etapas de trabalho consolidadas em 10 commits, organizadas por risco crescente. O destaque é o fechamento do princípio Open/Closed no sistema de blocos executáveis: adicionar um novo tipo de bloco agora exige apenas criarcm-<id>-block.tsx+<Id>FencedPanel.tsxe anexar entries emblockRegistry+blockPortals— zero edição emMarkdownEditor.tsx,markdown-extensions.tsoucm-slash-commands.ts.Net −970 LOC (+17.7k / −18.7k — a maior parte das deleções é código morto).
Gates (todos verdes, enforce mode)
prettier --checktsc --noEmitsize-check(≤ 600 L por arquivo tocado)coverage-check(≥ 80% por arquivo tocado)Zero arquivos com
// size:exclude file. Dois arquivos com// coverage:exclude filedocumentados in-line:execution-state.ts— módulo type-only (compila para.jsvazio; não há runtime para instrumentar)EnvironmentsPageContainer.tsx— o bloco de animação FLIP depende degetBoundingClientRect+requestAnimationFrame, que o jsdom retorna como0×0; cobertura visual é responsabilidade do projeto Playwright.Os 10 commits
coverage-check.shem commits só-deleção.lib/format/time.ts,blocks/execution-state.ts,backslashQuote,useConfigChangeRefresh).useConfigSyncedResource.useInlineForm+ConnectionFormreconstruído sobreuseReducer+ módulo puro de estado.HttpFencedPanel(2634 → 567 L) em view siblings + 4 hooks coesos + helpers puros + um state machine hook compartilhado.BlockTypeSpec/blockRegistry— fecha o gap Open/Closed.commands.tse oprettier --writenecessários para os gates de CI.CLAUDE.mdpara refletir a arquitetura pós-refactor.Decisões de não-unificação
Algumas unificações aparentemente óbvias foram avaliadas e descartadas porque os contratos divergem — forçá-las mudaria comportamento:
relative-time(HTTP / DB / history) têm contratos diferentes.DbFencedPanelnão consome o state machine hook compartilhado: seurunExplain+loadMoreco-mutam o mesmo estado querun; migrar exporia setters crus e quebraria a encapsulação.useMasterDetailgenérico: só 2 sites divergem, finamente — não compensa.Como revisar
Sugestão por risco crescente: commits 1–2 (bugfix + remoção de código morto) → commit 8 (Open/Closed) → commit 7 (decomposição do HttpFencedPanel) → commit 9 (apenas testes + 2 exclusões documentadas).
🤖 Generated with Claude Code