ci: per-check status badges + react-doctor workflow#236
Open
Kelbie wants to merge 38 commits into
Open
Conversation
Make `npx knip` run clean by acting on its real findings and silencing its false positives, verified individually rather than trusting the raw report. Removed (truly unreferenced): - functions: getReservedProofs, useNostrTierConfig, useNote, useNoteStats, selfIdentity - types: ComposerMode, UploadProgress Internalized (used only within their own file — dropped the `export`): - symbols: useMediaServerStore, DEFAULT_BLOSSOM_SERVER, fetchRelayInformation, buildSenderSelfCopyWrap, useTransactionAnnotationStore - 22 exported types across nostr/media, nostr/outbox, composer/publish, transactions, nip11, useEntityCache, transactionDistributionStore knip.json: - ignore `files` issue for *.ios / *.blur Metro platform variants (matches the existing *.android / *.liquid handling) — these are build-time entry points knip cannot resolve - add @sovranbitcoin/colada, nagg-ts, schemas to ignoreDependencies — linked siblings (scripts/link-local-siblings.mjs) that knip's resolver misreads as unused despite 80+ import sites Gates: knip clean, tsc --noEmit clean, eslint clean on touched files.
analyze-structure reported 2 import cycles; both were type definitions co-located with one side of a two-file pair. Extract each shared shape into its own module so neither file imports the other. - dmEnvelopeTypes.ts: DmEnvelope / DmEnvelopePage (was in dmEnvelopeClient, imported back by facadeDmAdapter). Client and adapter now both depend on the type module; client still imports the adapter's converters one-way. - recentContactTypes.ts: RecentContact (was in useNip17RecentContacts, imported back by mockDataStore). Hook and store now both depend on the type module. All importers repointed directly (ContactsScreen, dmDecryptPipeline, dmPagination, useDmConversations, useDmThread, mockDataStore, a test) — no re-export shims. Cycle count 2 -> 0; tsc and eslint clean.
loggerCore.ts was the repo's #3 complexity hotspot (cognitive 588, 898 LOC), mixing the emit/transport/ring-buffer core with two self-contained pure concerns. Move them out into deep modules behind small function seams: - loggerValueCompact.ts: the secret-redaction + value-summarization walk (classifyString, summarizeString, compactValue, field-name redaction, the SECRET/EMBEDDED/LONG string pattern tables). This is the redaction boundary; behavior is unchanged (pure move). - loggerSourceLocation.ts: stack-frame parsing (getCallerLocation, simplifyPath). loggerCore now imports compactValue/redactKnownSecretSubstrings/getCallerLocation and drops ~310 lines. Added __tests__/loggerValueCompact.test.ts unit-testing the extracted module directly (structural compaction + field-name redaction paths), complementing the existing end-to-end loggerRedaction suite. Gates: 41 logger tests pass, tsc clean, eslint clean (one pre-existing lazy-require warning unchanged).
contentShiftLog.ts (cognitive 579, 1401 LOC) mixed rectangle math with the registry/measurement/snapshot orchestration. Pull the pure primitives into contentShiftGeometry.ts (LayoutRect, validRect, rectBottom/Right, vertical/ horizontal/rectOverlap, viewportFlags) so the remaining file reads as orchestration over them. Registry/snapshot extraction deliberately deferred: VisualRecord ties the registry to the VisualLayoutConfig type web, so moving it would risk a new import cycle for marginal gain (the registry is a Map plus two helpers). Gates: tsc clean, 26 contentShift tests pass, eslint clean.
…ngine parseMethodParams was a nested function that never touched engine closure state (state/deps/generation/dedupe) — a pure pre-gate parse that validates per-method params and builds the approval preview, making no authorization decision. Lift it (and its ParsedSignParams result type) into nip46RequestParse.ts. The security-critical inbound pipeline (the 13-gate processEvent) is left fully intact and source-ordered in nip46Engine; only the pre-gate shape parse moved. Dropped the now-unused UnsignedEventSchema import from the engine. Gates: tsc clean, 783 nip46 tests pass, eslint clean.
The commit-3 extractions exported a few symbols that only their own module uses (verticalOverlap/horizontalOverlap in contentShiftGeometry, CompactOpts, SourceLocation, ParsedSignParams). knip flags them as unused exports; drop the `export` so the new modules expose only their real surface. Gates: knip clean, eslint clean.
…tConfig sovranPaymentConfig.ts was the repo's #2 complexity hotspot (cognitive 631, 1868 LOC) bundling six unrelated factory concerns. Split out the funds- execution core first: - sovranPaymentOperations.ts: createSovranExecuteReceive + createSovranExecuteMintQuote and their private helpers (receive-history polling, pending/fallback entry builders, ReceiveOperationLike). These hold the snapshot->poll->fallback race-window guards; moved verbatim, behavior-identical. - sovranPaymentLog.ts: the shared mintUrlLogFields redaction helper (used by operations, notifications, and handlers). Colada.tsx repointed to import the two operation factories from their new home (no re-export shim); dropped the now-unused imports from the config. sovranPaymentConfig drops ~480 lines. Notifications/handlers/scanSources/ screenActions decomposition follows in subsequent commits. Gates: tsc clean, payment-config tests pass, eslint clean.
Move createSovranScanSources (clipboard / gallery / NFC input sources) into sovranScanSources.ts — a self-contained concern with no shared payment-config state. Colada.tsx imports it from the new module. Also dedup: Colada.tsx had its own copy of mintUrlLogFields; point it at the shared sovranPaymentLog helper instead. Gates: tsc clean, payment-config tests pass, eslint clean.
…onfig Move createSovranNotifications (the ~460-line notification-code -> UI/state map) into sovranNotifications.ts. Pure relocation; imports pruned to the subset the notifications actually use. Colada.tsx and the profile test repointed to the new module (no re-export shim). Gates: tsc clean, payment-config tests pass, knip + eslint clean.
…Config Move createSovranScreenActionHandlers (post-terminal NFC/emoji/copy/memo actions) and its Ctx narrowing helpers (sendCtx, mintQuoteCtx) into sovranScreenActions.ts. injectRecipientPubkey stays in the config file — it is a handlers helper that merely lived in the screen-actions section. Colada.tsx repointed to the new module. After this, sovranPaymentConfig.ts holds only createSovranHandlers + its helpers (down from 1868 to ~570 LOC). Gates: tsc clean, payment-config tests pass, knip + eslint clean.
With operations, notifications, scan sources, and screen actions extracted, the file holds only createSovranHandlers (+ its helpers), so the "PaymentConfig" name no longer fits. Rename to sovranHandlers.ts, repoint the two importers (Colada.tsx, profile test), and rewrite the file header. The 1868-LOC #2 complexity hotspot is now six focused modules: sovranPaymentOperations, sovranNotifications, sovranScanSources, sovranScreenActions, sovranHandlers, and the shared sovranPaymentLog. Gates: tsc clean, payment tests pass, knip + eslint clean.
useMintRebalanceOrchestrator's pure decision logic was already extracted (rebalanceRunState + rebalance helpers, both tested); the remaining inline logic is closure-coupled orchestration. computeRouteSuggestion was the one exception — a pure-ish failure-recovery computation (gather candidates -> audit -> build graph -> pick path) that only depended on its inputs plus an auditor lookup. Move it to rebalanceRouting.ts behind an injected `fetchAudit` port (and passed-in groups), so it's unit-testable without network/store access. The hook keeps a thin closure that gathers values and delegates. Added __tests__/rebalanceRouting.test.ts (5 cases: null path, name mapping, candidate dedup, audit filtering, 12-candidate cap). This is on the reroute path, NOT the live melt-execution path — the funds-critical executeStep orchestration is untouched. Gates: tsc clean, rebalance tests pass (15), knip + eslint clean.
mintUrlLogFields — the mint-URL log redaction helper ({ hasMintUrl,
mintUrlLength }) — was copy-pasted byte-identically into 37 files across
app/, features/, and shared/ with no owner. Classic latent duplication.
Hoist it to shared/lib/mintUrlLog.ts as the single owner, migrate all 37
sites to import it, and delete every local copy. Also retire the interim
features/send/lib/sovranPaymentLog.ts (introduced earlier this branch) and
point its consumers at the canonical module — no re-export shim.
The body is unchanged everywhere, so redaction behavior is identical; this
just removes the duplication and gives the helper a home.
Gates: tsc clean, knip clean, eslint clean, full suite green (1664 tests,
150 suites).
describeImageLoadError — the <Image> onError-event-to-string normalizer — was copy-pasted byte-identically in WallpaperThumbnail, UnitPreviewCard, and SpriteView. Hoist to shared/lib/imageLoadError.ts and migrate all three to import it; delete the local copies. Behavior identical. Gates: tsc clean, knip clean, eslint clean, full suite green.
The useAuditedMint(s) and use{Nostr,Sovran}DiscoveredMints hooks were
carried since the shared/features migration but never wired into any
screen and had no tests — they survived only as mint-barrel re-exports.
Removing them orphans their sole dependency, the NIP-87 recommendation
helpers in shared/lib/nostr/client.ts (parseRecommendation,
isCashuRecommendationEvent, extractMintUrlFromEvent, NostrEvent), so
those go too; npubToPubkey stays. Also prunes the dead rebalance/lib
re-exports from the mint barrels and un-exports types used only within
their own module.
Prunes re-export lines that nothing imports from the feed, transactions, wallet, user, camera, settings, image-overlay, popup, MintSelector, GlassSearchBar and QRButton barrels, and removes the dead `export default LoadingIndicator` (every consumer uses the named export) — clearing the default+named clash and a duplicate export. Interfaces and types used only within their own file are un-exported rather than deleted; a handful of genuinely dead functions (getFirstTagValue, feedRowsAreEqual, getEmbeddedRepostEvent, normalizeFeedEvent, readThreadSeed, the exported useTransactionSource duplicate, setPopupDuration, the unused liquidGlassModifiers helpers, the unused PullToAiRefreshControl component) are removed.
`__tests__/**` and colocated `*.test.*` are now entry points, so test-only-used code is no longer mis-flagged as dead. With the preceding cleanup, the ~20 broad per-tree `ignoreIssues` masks (features/feed, features/mint, shared/lib/popup, shared/styles/tokens, …) are gone — knip is green on its own. What remains is structurally justified: the four platform-variant `files` patterns (Metro resolves them by extension), the codereview tool API, the dynamic icon registry, the vendored p2pk plugin, and three narrow file-scoped suppressions for surfaces knip can't statically see (naggSchemas + recentPeopleProfiles, in-flight graphql-strip consumed via dynamic import; sendMemoSheet helpers consumed by a require()-based test).
The structural analyzer parsed only static `import … from` / `export … from`
edges, so a module reached ONLY via `await import('…')`, `void import('…')`,
or `require('…')` was reported as a dead orphan, and a symbol pulled only via
`const { x } = await import('…')` was reported as an unused export. Both are
false positives — the code is live, the bundler resolves it, and knip already
counts it as used.
extractImports now also records dynamic-import and require edges (module plus
destructured binding names). They are tagged `isDynamic` and feed ONLY the
usage/reachability views — fan-in *presence* (orphans) and imported-names
(unused exports) — and are deliberately excluded from every static-coupling
metric: cycle detection (a lazy import is exactly how a static cycle is
broken, so counting it would forge cycles), fanout / importer-reach, and the
fan-in *magnitude* used by hub-spoke and pass-through. The change is therefore
surgical: nothing that measures static structure moves.
Verified on this tree (analyze-structure --history --reach --leakage
--vocab-drift): dead-orphan files 5→4 (e.g. shared/lib/profile/ownProfileSync,
loaded via `void import()` in NostrKeysProvider; the two color scripts
require()d by build-background-themes.js); unused-export files 26→23
(purgeFixtureMetadata via require in settingsStore, clearAllSecureData via
dynamic import); cycles still clean; hub-spoke (141), pass-through (43) and all
other metrics unchanged; Overall 61→62.
Address the highest-signal React Doctor findings and seed representative samples for the migration-scale rules (full sweeps deferred to follow-up). Full fixes: - Security/build-pipeline-secret-boundary: install with --ignore-scripts while the GitHub Packages token is in scope (it is only needed to auth the registry fetch), then run trusted dependency scripts (skia) and the first-party postinstall in a separate token-free step. Applied to both CI jobs. - Perf/no-json-parse-stringify-clone: structuredClone in the loggerChild test. Representative samples (recipe confirmed; remaining sites need sign-off): - react-compiler-no-manual-memoization: drop redundant useCallback in the home + notifications layouts, hoisting the closure-free handler to module scope. drawer/_layout.tsx:131 left intact: its React.memo is a load-bearing render-bailout for drawer animation timing (rule false positive). - set-state-in-effect: derive isChecking from permission in the camera hook. - js-combine-iterations: single-pass for...of in useAllSearchResults and pollParse. - no-giant-component: split PostComposer (405->300 lines) into Header, MediaTray and Toolbar sub-components, preserving the VisualLayoutProbe log taxonomy. Deferred: refs (timing-sensitive), plus the full sweeps of the above rules. Verified: react-doctor 2857->2847 with no new findings; type-check, knip, loggerChild test, and prettier (changed files) all pass.
React Compiler is enabled (app.json experiments.reactCompiler), so manual memo/useMemo/useCallback the compiler already covers is dead weight. An AST codemod unwraps ONLY React's memo/useMemo/useCallback (and removes the now-unused imports), across 125 files — 360 wrappers removed. Kept as load-bearing (verified against a fresh react-doctor run + cross-model review, NOT swept): - files where removal introduced a new compiler bailout (preserve-manual- memoization / refs / immutability / purity / no-effect-with-fresh-deps) - a side-effecting useMemo factory (NDKCacheAdapterSqlite in NostrNDKProvider) - memos with explicit human justification (stable prop identity, list-row render bailouts) The rn-no-non-native-navigator (x3) and js-length-check-first (x2) findings from the same react-doctor pass are confirmed false positives for this Expo Router setup and were intentionally left untouched: no native drawer exists in react-navigation v7, and the flagged array compares are an intentional prefix match and an already-guarded equality check. react-doctor total 2847 -> 2507; react-compiler-no-manual-memoization 1320 -> 960 (-360); all compiler-health rules flat. type-check, eslint (0 errors), and 1664 tests pass.
Audit of the repo-root config/dotfiles, cross-checked with a second model. Remove five files that no longer have a load path: - app-env.d.ts / nativewind-env.d.ts: redundant `uniwind/types` references (nativewind is no longer a dependency; the app migrated to uniwind). The reference is already provided by the uniwind-generated uniwind-types.d.ts, and both were byte-identical leftovers. - components.json / tailwind.config.js: shadcn CLI config plus the Tailwind JS config it pointed at. shadcn is not a dependency, Tailwind v4 does not auto-load a JS config without an `@config` directive (and global.css has none), and uniwind never reads it. The colors it defined already live in global.css `@theme`. - .DS_Store: macOS cruft (already gitignored, was untracked). Drop the now-dead "nativewind-env.d.ts" entry from tsconfig `include`; the `**/*.ts` glob still covers the remaining uniwind types. type-check passes; lint shows only pre-existing prettier warnings in unrelated files.
…mbine-iterations) Three genuine app-runtime list transforms over medium/large arrays — thread replies, a notifications page, and the swap-group timeline — rewritten from .map().filter() / .filter().map() to a single .flatMap() pass (the rule's own canonical fix). Identical semantics, comparable readability, one fewer traversal and intermediate allocation each. Scoped, cross-model-validated sample (me + Codex agreed): the remaining js-combine-iterations sites and ALL js-set-map-lookups sites are false-positive-dominated in this RN/Expo codebase and were left as-is — offline codereview/scripts tooling (not in the app bundle), string-receiver misfires (msg.includes / field.indexOf), and sub-10-item or already-memoized arrays where a second pass is negligible.
Adds devDependencies for the next-tier lint rules: neverthrow must-consume-result (@okee-tech), no-secrets, security, unicorn, sonarjs, and a direct dep on eslint-plugin-import (previously only transitive via expo). No rules enabled yet.
Replace the three object-literal `{...} as RoutstrError` throws with a
`makeRoutstrError` factory (satisfies consistent-type-assertions without
a cast) and narrow `per_request_limits: any` to `unknown` (the field is
never read).
Replaces opaque eslint-suppressions.json counts with documented, scoped exceptions for genuinely-legitimate cases: - redux/**/*.deprecated.ts: no-explicit-any off (live migration scaffolding reading arbitrary old persisted Redux blobs; slated for deletion). - routstr/api.ts: no-restricted-globals off (every endpoint needs the raw Response for its status-aware error pipeline + SSE streaming; fetchJson cannot carry that contract). - Button/SpriteView/AmountFormatter: no-restricted-imports off with a TODO(reanimated-migration) note (legacy Animated, deferred to its own PR). - easeGradient.ts: inline disable (MIT adaptation driving RN's internal color-interpolation API; no Reanimated equivalent).
Clears the no-restricted-syntax (hex literal) suppressions: - White-on-dark icons/text -> INVARIANT_WHITE; immersive/story/shadow blacks -> INVARIANT_BLACK. - New shared brand constants for genuinely cross-site values: LIKE_ACCENT (#ff5a7a), AMBER_ACCENT (#f59e0b), WALLPAPER_PLACEHOLDER gradient. - Color-definition homes (map/categories marker palette, useThemeColor resolver fallback, RowStatsAccent STAT_COLOR_* exports) added to the no-restricted-syntax exempt list alongside themes.ts/brandColors.ts. - Localized intentional invariants (notification-type palette, native-tab theme colors, default decorative gradient, avatar placeholder fill) get documented inline disables instead of opaque bulk-suppression counts. No rendered colors change: every swap maps a literal to a constant of the same value or an existing token of the same value.
The eslint-disable-next-line directive sat above the explanatory comment block, so it targeted a comment line instead of the restricted import. Move the directive to the line immediately before the import.
Clears the no-explicit-any suppressions for live (non-deprecated) code: - Concrete types where derivable: forwardRef<ElementRef<typeof View>>, StyleProp<ViewStyle>, a structural Measurable ref type (hero-transition), typed routstr stream via Awaited<ReturnType<...>>, Href for the router pathname, the reanimated scroll-handler type, typed mint-info/theme-registry records, and narrowed object casts. - Redundant casts dropped: the (delta as any) chat-stream casts are covered by the zod chunk spine; Supercluster <any,any> generics fall back to the library's AnyProps default. - Documented inline disables only where any is genuinely load-bearing: typedUpdate's implementation signature (behind two typed overloads), migrateSettings reading a legacy Redux blob, gorhom/bottom-sheet render-prop props, react-native-image-colors' platform union, the compose() provider util, and the Animated.FlatList ref typing gap.
Clears the consistent-type-assertions suppressions (object-literal casts): - Typed seams instead of casts: reduce<Record<string,number>>, typed .catch arrow returns (BalanceMap / byMint result), satisfies for the NDK->Applesauce event and the custom-sheet payloads, drop the redundant useMemo<T> inner cast. - Drop unneeded spread-copy cast in PaymentRequestScreen. - Assert the merged variable (not the literal) in createMergeWithSchema where TS can't prove a generic spread is complete. - Documented inline disables for the two genuinely-load-bearing cases: the generic TActiveChildren empty default and gorhom's undocumented useDirectView contentContainerProp.
…file Enables the researched high-signal rules, drives their backlog to zero (or to warn), and removes eslint-suppressions.json ENTIRELY (193 suppressed violations -> 0; the file is deleted, not emptied). Also includes the mechanical --fix output the autofixable rules produced (prettier, optional-chain collapses, import dedupe, dead-import removal) — no behavioural changes there. error tier (backlog driven to zero): - await-thenable, prefer-optional-chain - unicorn: no-thenable, no-instanceof-array, error-message, throw-new-error - sonarjs: no-identical-expressions, no-all-duplicated-branches, no-element-overwrite, no-collection-size-mischeck, non-existent-operator - import/no-duplicates, import/no-self-import - security: detect-pseudoRandomBytes, detect-eval-with-expression, detect-bidi-characters - (the 4 remaining optional-chain + 2 Error-message sites fixed by hand) warn tier (visible, never suppressed): - neverthrow/must-consume-result - no-secrets (tuned tolerance + ignore Nostr/Cashu encodings; off in fixtures) - import/no-cycle (depth-gated) - switch-exhaustiveness-check (~18 genuine gaps -> focused follow-up) - prefer-nullish-coalescing (||->?? needs a falsy-0/'' audit before promotion) eslint-plugin-import promoted to a direct dependency. Per the no-new-suppressions rule, every adopted rule either finished at zero or landed at warn.
tsc --noEmit surfaced 15 errors where a removed `any`/cast was load-bearing: - Colada: the useMemo<MachineOperations> object needed both the assertion AND contextual param typing — extract to a variable, assert that, and type the one inline operation's params explicitly. - useAiSend catch, list.tsx router pathname, Supercluster Options generics: the value is genuinely untyped/3rd-party — restore `any` with a documented inline disable rather than a wrong type. - PaymentRequestScreen: double-assert through unknown (types don't overlap). - compose(): guard the reduce accumulator off undefined instead of `as any`. type-check and lint both clean (0 errors).
…port - eslint-plugin-import is provided transitively by eslint-config-expo and is referenced via import/* rule names (not require()d), so the direct devDependency is redundant — knip flagged it. The import/no-duplicates, no-self-import and no-cycle rules still resolve through expo. - Drop the unused `export` on the RelayHealth type (used only internally; pre-existing knip finding, fixed here so the gate stays green).
React Compiler is enabled, so manual useMemo wrapping a cheap pure value is dead weight it already covers. Commit 584186b removed the codemod-safe 360 (1320->960); this pass clears 70 of the residue it left behind — value memos stranded in files a coarser per-file filter preserved. Method: unwrap only single-line `useMemo(() => expr, [deps])` value memos (no React.memo wrappers, no useCallback, excluding animation/gesture/Skia/ shared-value expressions), then verify each batch against a fresh `react-doctor --scope changed` run. Reverted every removal that introduced a new compiler-health finding rather than force it through: - targetItem (ThreadView) fed a kept useCallback -> whole-component bailout. - PollCard / NearPayScreen / LightningStrike / SkeletonExitShimmer memos fed effects -> no-effect-with-fresh-deps (effect re-runs every render). - SettingsDesignSystemTimelineScreen -> purity. react-doctor's per-removal scan catches these value/callback bailouts (preserve-manual-memoization / no-effect-with-fresh-deps / purity); it does NOT catch React.memo render-bailouts (verified: removing the load-bearing MenuButton memo produced no finding), so component wrappers are left alone. The rn-no-non-native-navigator (x3) and js-length-check-first (x2) findings from the same pass remain confirmed false positives and untouched: RN v7 has no native drawer, two of three nav sites are type/hook imports, and the flagged compares are an intentional prefix match and an already-guarded equality check. react-compiler-no-manual-memoization 960 -> 890. type-check 0 errors, eslint 0 errors / 0 new warnings, 1664 tests pass (one source-assertion test updated to match the unwrapped contactRows line).
React Compiler (app.json experiments.reactCompiler) auto-memoizes values, callbacks, and component output, so most manual useMemo/useCallback is redundant. The react-perf/jsx-no-new-* eslint rules were deliberately paired with the compiler to flag prop instability — which directly contradicts deleting those memos. Make the compiler the source of truth: remove the three react-perf rules + plugin + devDependency, then sweep the safe subset of react-compiler-no-manual-memoization findings (890 -> 580). Bounded to compiler-covered cases only. Kept every React.memo wrapper, preserve-manual-memoization site, commented memo, and generic useMemo<T>/useCallback<T> (dropping the type arg breaks inner-param inference). Skipped any binding consumed by a hook dependency array (removing it re-fires the effect every render) and multi-statement block useMemo. Verified per-file with `react-doctor --scope changed` and a full before/after per-(file,rule) count delta against the original report: total issues 2434 -> 2121, zero net-new issues of any rule. tsc, eslint (0 errors), and knip all clean. 16 files with genuine new findings were reverted wholesale. Left two false-positive clusters untouched (see ADR 0006): rn-no-non-native-navigator (fix target @react-navigation/native-drawer does not exist; sites are a deliberate expo-router/drawer surface, a useDrawerProgress hook, and a type-only BottomTabBarProps import) and js-length-check-first (both are the rule's own documented exceptions: an existing length guard, and an intentional prefix-match).
Move render-stable helper functions out of component bodies to module scope. None captured component state/props — only their own params, module imports, and module consts — so React rebuilt them on every render for nothing, and a fresh function identity defeated memoized children. Pure relocation; zero behaviour change (the compiler already stabilised these, so their location never affected runtime). Clears all 30 react-doctor prefer-module-scope-pure-function findings. MerchantDetailScreen's handleCall/handleEmail/handleContactPress and SearchResultRows' renderItem were hoisted as cascade follow-ups once their sole dependency (handleOpenURL / renderSearchResult) moved out. Verified against the real tool: prefer-module-scope 30 -> 0, total react-doctor 2121 -> 2012, zero net-new findings of any rule; tsc, eslint (0 errors), and knip all clean.
The first React-Compiler memo sweep (584186b) removed ~20 React.memo/memo() wrappers and never restored them — including the app's most render-sensitive components (charts, animated backgrounds, gradient overlays, feed media, transfer/transaction list rows). React.memo is a props-comparison RENDER bailout, a different mechanism from value memoization; the compiler does not reliably replace it (React Compiler 1.0 guidance, point #2). The "keep all wrappers" rule was only adopted in pass 2 — pass 1 had already stripped them. Runtime logs confirmed the regression: a log-doctor renders pass showed the three instrumented stripped components re-rendering repeatedly (tx.monthly_chart 6x, bg.view 7x, ScrollableGradientOverlay 4x) — the user-reported lag. Changes: - Re-wrap all 20 stripped components in React.memo/memo(). - Restore getMintInfo's useCallback([manager]) in useMintManagement — a callback consumed as a useEffect dependency in CONSUMER hooks (useMintInfo.ts, useMintContacts.ts); the sweep's "skip dep-array bindings" guard only checked the same file, so this cross-file escape hatch slipped. - Add a React Compiler coverage gate: scripts/check-react-compiler.mjs + check:react-compiler npm script + CI step. It runs react-compiler-healthcheck and fails when compiled < total. The eslint-plugin-react-compiler rule that would normally catch a bailout is dead under the repo's zod@4 override, so this restores the signal using the compiler itself as source of truth. - ADR 0007 documents the finding + gate; corrects ADR 0006's "kept every wrapper" claim. Verified: react-compiler-healthcheck 596/596 (zero bailouts), tsc 0, eslint 0 errors, knip 0, jest pass, prettier clean.
Second audit pass over the React-Compiler memo sweep diff. - Restore 3 gesture useMemos the bulk pass (8bd4f74) stripped despite the earlier passes' explicit policy to exclude gesture/animation memos: ThreadEmbedSheet/DistributionSlider/NearPayScreen panGesture+gesture. A fresh Gesture.Pan() each render re-registers the GestureDetector handler — a footgun the render-count log wouldn't surface. Re-wrapped with their original deps. - Cut the two over-logging events that were 61% of a sample session's log volume (the user-reported "repeated logs"), neither a memo bug: * mint.info.fetch.success (31%): the hook re-logged success on every getMintInfo call including instant SWR cache hits, duplicating the cache layer's own store.mint_info.* hit/miss/fetch logs. Removed the redundant hook-level start/success debug logs (kept the error log). * nostr.client.npub_to_pubkey (30%): a debug log on every call of a pure decode invoked per-row across follower/contact lists. Removed (kept decode-failure). - Restore addQuery's useCallback (useRecentSearches) — consumed as a cross-file useEffect dep; deps [addSearch, surface] are stable. Context-provider value memos were investigated and deliberately left compiler-owned: restoring them introduced 7 net-new react-hooks/exhaustive-deps warnings (their callback deps aren't manually memoized), and the compiler already stabilizes them (596/596 compile) under the new coverage gate. See ADR 0007 "Second pass". Verified: tsc 0, eslint 0 errors, knip 0, jest 1664/1664, prettier clean, react-compiler-healthcheck 596/596.
Split lint, type-check, knip, and React Compiler coverage out of the single code-quality job into dedicated workflows so the README can show a status badge per check (GitHub Actions badges are workflow-scoped, not per-step). Add a non-blocking React Doctor workflow that runs `bunx react-doctor@latest --verbose` and uploads the report as an artifact — informational signal, kept off the merge gate given its false-positive rate on this codebase. Extract the shared Bun setup + secret-boundary install (deps:sovran-latest, --ignore-scripts auth fetch, then trust + postinstall with no secret in scope) into a composite action (.github/actions/setup-sovran) reused by every gate workflow, with a Bun-store cache so the per-check split doesn't pay the install cost five times over. ci.yml keeps Prettier, Test, and the structural-health signal job. Add the `react-doctor` npm script for local parity and a badge row to the top of the README.
Structural health |
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.
What
Adds a status badge to the top of the README for each CI check — Lint, Type Check, Knip, React Compiler, and React Doctor — and the workflows needed to back them.
GitHub Actions badges are workflow-scoped (a badge URL resolves to one workflow file's status, not a job or step), so a single bundled
ci.ymlcan only produce one "CI" badge. To get a badge per check, each check now lives in its own workflow file.Changes
.github/actions/setup-sovran— new composite action holding the shared Bun setup + secret-boundary install (deps:sovran-latest→bun install --ignore-scriptswith the registry token in scope →bun pm trust+postinstallwith no secret in scope), plus a Bun-store cache so the per-check split doesn't pay the install five times over. Reused by every gate workflow.lint.yml/type-check.yml/knip.yml/react-compiler.yml— each runs one gate → one badge. React Compiler coverage was previously only a step insidecode-quality; it now also has a dedicated badge.react-doctor.yml— runsbunx react-doctor@latest --verbose, non-blocking (continue-on-error), and uploads the verbose report as an artifact. It is informational signal, deliberately kept off the merge gate given its false-positive rate on this codebase; the badge stays green to mean "it ran".ci.yml—code-qualityandstructural-healthnow use the composite action;code-qualitykeeps only the gates that don't need a dedicated badge (Prettier, Test). No gate runs twice.package.json— adds thereact-doctorscript for local parity.README.md— badge row under the title.Notes
chore/eslint-suppressions-and-rules; until that merges, this PR's diff includes its commits. The composite action intentionally matches that branch's two-phase secret-boundary install.