The single biggest source of `any` in this codebase: react-force-graph
is a loosely-typed library by design (NodeObject/LinkObject both carry
`[others: string]: any` index signatures), and every hook/component
touching the graph ref, node/edge callbacks, or force settings had
absorbed that looseness as `any` instead of narrowing it once at the
boundary. Fixed the boundary, not each site individually — most of
this diff is mechanical once the shared types existed.
New shared types (types/graph.ts):
- GraphViewerRef = ForceGraphMethods<NodeObject<GraphNode>,
LinkObject<GraphNode, GraphEdge>> | undefined — the one ref type
every graphRef across 8 files now uses instead of re-guessing
`React.RefObject<any>` at each site.
- GraphNode gained fx?/fy? — react-force-graph's own NodeObject type
declares these (pin coordinates set after drag-end and after
layout), and every consumer that touched them was already doing so
through `any`; the app's own type just never caught up.
Root-cause fix in index.tsx: explicitly parametrizing
`<ForceGraph2D<GraphNode, GraphEdge>>` (it was rendered with no
generics, defaulting every prop's NodeType/LinkType to `{}`) made
onNodeClick/onNodeRightClick/onLinkClick/onLinkRightClick/
nodeCanvasObject/linkCanvasObject all correctly typed against the
app's real node/edge shapes for free — four `any` params across two
files resolved by one annotation, not four separate patches.
Found and fixed a real TS gotcha this surfaced: GraphEdge declares
source/target as plain required strings, which collides with
LinkObject's own optional `source?: string | number | NodeObject`
when GraphEdge is used as the LinkType generic — the intersection of
a required string and that optional union collapses to `never` once
narrowed with `typeof x === 'object'`, silently breaking any code that
reads `.x`/`.y` off a link's resolved endpoint. Verified in isolation
before touching the real files (see the two `LinkObject<GraphNode,
Omit<GraphEdge, 'source'|'target'>>` sites and their comments). Every
earlier place in this codebase handling this exact duality with hand-
rolled `typeof === 'object' ? x.id : x` guards was working around this
same underlying incompatibility without naming it.
Found and removed real dead code exposed by typing, not introduced by
it: link-renderer.ts's LinkRenderParams declared theme/highlightNodes/
selectedEdges and both call sites dutifully threaded them through, but
renderLink's body never reads any of the three — their information was
already baked into `rc` (RenderContext) before the call. Confirmed by
literal absence from the destructure, not inferred; removed from the
interface and both call sites.
Documented, not silently patched: use-graph-initialization.ts's
getViewportCenter called `graphRef.current.getBoundingClientRect?.()`
— a DOM method ForceGraphMethods doesn't have, so the optional call
was always undefined and the function always returned the {x:0,y:0}
fallback. Feeds "center new item in viewport" (add-item-dialog.tsx),
which has therefore always placed new items at graph-origin, never the
actual visible center. Left the behavior unchanged with the bug
explained inline — fixing it needs the container's DOM rect, which
this hook isn't given, and guessing at a fix here without a way to
verify it against the running app risked a worse, unverifiable
regression.
Also typed through the two hooks/use-layout.ts and
hooks/use-graph-layout.ts (nodes/graphData/forceSettings/
saveAllNodePositions, matching the same real types), and the worker
message boundary in use-layout.ts (a minimal `{id, x?, y?}` type for
what crosses postMessage's structured-clone, since that's genuinely
all that's read off it).
Verified: eslint no-explicit-any 257 -> 202 in one pass, tsc 89
(unchanged from the running baseline — every step diffed against it,
not just "count went down"; two remaining findings are the
established table/data-table.tsx dead code), vitest 15/15, yarn build
clean. Also fixed two no-extra-semi findings in nodes-view.tsx/
relationships-view.tsx that kept reappearing: deleting the leading `;`
alone doesn't stick because prettier's own ASI-safety rule re-inserts
it on every format pass when a statement starts with `(` — restructured
to a local `const` instead of leaving something prettier would keep
fighting.
NodeProperties/NodeMetadata: [key: string]: any -> unknown. The data
itself is genuinely dynamic (backend-driven custom-type schemas, no
fixed shape ever exists), so the honest type still has an index
signature — but unknown forces every read to narrow before use instead
of silently trusting whatever shape shows up. Verified empirically:
switching surfaced exactly 3 real call sites relying on implicit any
(nodes-view.tsx's created_at read, and two details-panel.tsx fields
feeding TagsInput/array ops) — fixed each with a cast at the point of
use, matching what the code already assumed. Documented both types
with why they stay index-signature-shaped rather than a fixed
interface.
GraphNode.neighbors/links: any[] -> a minimal shape matching what every
actual consumer reads (neighbor.id; link.source.id/link.target.id).
Not GraphNode[]/GraphEdge[] — neighbors briefly holds react-force-graph's
own runtime node objects mid-transform, and a fully accurate type would
need to be self-referential for a benefit nothing here uses. links'
source/target are typed `string | { id: string }` because that's the
real runtime duality already handled elsewhere in this codebase (see
lib/utils.ts's `typeof edge.source === 'object' ? ... : ...` guard) —
react-force-graph mutates a plain edge's source/target from an id
string into a node-object reference once it resolves the graph, and
different call sites see it before or after that mutation.
Fixed the actual any-typed consumer while I had the real types
available: use-highlight-state.ts's handleNodeHover/handleLinkHover
were `(node: any)`/`(link: any)`, and handleNodeHover's own links loop
read `link.source.id`/`link.target.id` directly with no typeof guard —
inconsistent with handleLinkHover right below it, which already
handled the string-vs-object duality correctly. Extracted a shared
linkEndpointId() helper and used it in both, closing that latent gap
(would have been `undefined-undefined` in the highlighted-links set
whenever a link's endpoints hadn't been mutated to objects yet).
tools.test.ts: one test constructed a `links` entry with only `target`,
no `source` — asserting against a stub function
(getListOfAllChildrenFromNodes does nothing, param unused) that
happened not to care, but not a valid GraphNode.links entry. Completed
the test data instead of loosening the real type to accommodate it.
Also fixed one unrelated no-extra-semi in nodes-view.tsx (same
`;(el as HTMLInputElement)...` pattern fixed in relationships-view.tsx
in an earlier commit — missed this file's copy of it then).
Verified: tsc 89 (unchanged from before this change — every
consumer-site fix confirmed by diffing tsc output against that exact
baseline, not just "count went down"), eslint clean on all five
touched files besides pre-existing no-explicit-any warnings elsewhere
in two of them, vitest 15/15 (tools.test.ts's 5 tests included and
passing), yarn build clean.
One tool instead of three, one config instead of four duplicated
blocks. black/isort/flake8 dropped from every package's dev deps,
ruff added once; .flake8 deleted, [tool.black]/[tool.isort] blocks
deleted from all 4 sub-package pyproject.toml files.
Config lives once, at the workspace root: ruff walks up from any file
it's checking until it finds a pyproject.toml with a [tool.ruff]
table (verified empirically — a package's own pyproject.toml lacking
that table doesn't stop the search), so the root-level [tool.ruff]
applies to all four packages without duplication. That's the one real
win over the old per-package black/isort blocks, which had to be
copy-pasted four times and kept in sync by hand.
Same rule surface as before, not more: select = E/F/W/I (pycodestyle
+ pyflakes + isort-equivalent), matching exactly what flake8 checked.
ruff has other rule groups (UP/pyupgrade, B/bugbear) that would
surface real things — deliberately left off. Turning those on finds
new debt and is a separate decision with its own diff to review, not
a side effect of swapping the tool underneath an unchanged rule set.
Same E501 exclusion carried over too, same reasoning: the formatter
is the sole authority on line length.
known-first-party carries over unchanged (same isort concern this
repo already fixed once — see "app" vs "flowsint_api" in the ratchet
history) and the wildcard-import per-file-ignore for
flowsint_core/__init__.py still applies, verified directly.
Six files reformatted by `ruff format` where it disagreed with black
on edge cases (import grouping, ternary line-breaking, blank lines
after a class statement) — all cosmetic, no behavior change, reviewed
diff by diff. One real ruff-check finding (an import block ruff's
isort-equivalent wanted sorted differently) — took ruff check --fix.
Verified: `make lint` clean, `make test` green (all 4 packages),
lint-staged tested with a real staged-file run (not just config
inspection) to confirm `uv run ruff format` / `uv run ruff check --fix`
actually execute, mypy ratchet target unaffected (still fires on its
own pre-existing backlog, unrelated to this change).
graph-settings-store.ts: forceSettings and every settings-lookup
helper (getSettingValue, getCategorySettings, etc.) were typed `any`
even though the store's own DEFAULT_SETTINGS constant gives them a
real, inferable shape. Added SettingItem (the shape every entry under
a settings category shares) and GraphForceSettings (= typeof
DEFAULT_SETTINGS.graph, since that's literally what's assigned as the
initial value) and threaded them through updateSetting, getSettings,
getCategorySettings, and applyPreset. The migrate() function's
persistedState stays untyped on purpose — it's whatever an arbitrary
older app version wrote to localStorage, genuinely unknown, matching
zustand's own persist types.
types/graph.ts: deleted ForceGraphSetting, GeneralSetting,
ExtendedSetting, and Settings — zero consumers anywhere in src. Not
narrowed, removed: keeping unused `any`-shaped types around just
invites new code to reach for them instead of the real settings-store
types above.
getSettingValue's return type widened from implicit any to its real
`number | boolean | undefined` — every setting in DEFAULT_SETTINGS is
one of those two. Four call sites (autoZoomOnCurrentNode,
autoColorLinksByNodeType, showBackground, showMinimap) treat that
value as a boolean; wrapped each in Boolean() rather than leaving the
union unresolved, since every current caller only checks truthiness.
Found and removed one dead prop along the way: index.tsx was passing
warmupTicks to ForceGraph2D by reading forceSettings?.warmupTicks —
but that setting is commented out in DEFAULT_SETTINGS and has been for
a while, so the read always fell through to its `?? 0` fallback.
ForceGraph2D already defaults warmupTicks to 0 on its own, so the prop
was a no-op; removed instead of typing a field that doesn't exist.
Verified: tsc 89 (unchanged vs. the last known-good baseline — the two
remaining line-number-only diffs are pre-existing, unrelated to this
change), eslint 0 errors on the four touched files (12 pre-existing
no-explicit-any warnings remain in graph.ts's NodeProperties/
NodeMetadata/neighbors/links, next in this pass), vitest 15/15, yarn
build clean.
Two rules disabled in .eslintrc.cjs, not per-site fixed — both are the
lint config not matching this stack, not real defects:
- react/prop-types (113): this is a TS codebase, every prop is already
validated at compile time by its interface. The rule exists for
plain JS React with no type system; requiring PropTypes.* on top of
TS types would duplicate what TS already checks, everywhere.
Standard practice in TS+ESLint setups (CRA's TS template,
eslint-config-airbnb-typescript, etc. disable this too).
- react/display-name (32, 31 in live code): audited every finding —
each one is `const Name = memo((props) => {...})` or
`forwardRef<T>((props, ref) => {...})`, a real component with a real
name via its binding. Documented eslint-plugin-react limitation: it
can't infer a displayName through memo()/forwardRef() wrapping an
anonymous arrow function, even when assigned to a named const. Not
the same as a genuinely nameless component (one defined inline with
no binding at all) — those are real bugs, already fixed under
react-hooks/static-components. Renaming ~30 arrow functions to
named function expressions purely to satisfy this blind spot isn't
worth the diff churn.
Then the real remaining findings:
- 7x missing `key` prop on list items (.map() rendering Link/li/
SelectItem/DropdownMenuRadioItem without one) — genuine risk of
state bleeding across list items on reorder/filter, not style.
Added stable keys (entity id where available, composite key where
no single id field exists).
- 2x react-hooks/refs regression I introduced in the
set-state-in-effect commit: details-panel.tsx's node-sync block was
converted from a useEffect to a render-time adjustment, but it also
mutated formDataRef/nodeSizeRef directly — illegal during render,
and the lint run after that commit should have caught it but didn't
get checked against this specific rule. Fixed by extracting a
standing useLayoutEffect that keeps both refs in sync with their
state on every render (matches the "always read latest value"
intent the refs already existed for), and removing the two
render-time ref writes now that the effect covers them.
Verified: eslint clean outside data-table.tsx (established dead code)
and tutorial-provider.tsx (established dead tour feature), tsc 89
(unchanged), vitest 15/15, yarn build clean.
Follow-up to the react-hooks/refs commit: the useLayoutEffect I added
there to measure --editor-width has no dependency array on purpose —
that CSS custom property has no React-observable trigger, so
re-reading it after every commit is the only way to stay current
(matches what reading it directly during render used to do, before
that read got moved off the render path for being an unsafe DOM
access). exhaustive-deps flagged it as a potential infinite-update
risk, which it can't rule out. Disabled with the reasoning inline
rather than inventing a dependency that doesn't exist.
Verified: eslint clean on this rule outside the pre-existing dead
table/data-table.tsx findings, tsc 89 (unchanged), vitest 15/15, yarn
build clean.
Last five rules, 18 findings (4 in table/data-table.tsx — dead code,
left alone). Two were real bugs, not lint noise:
- flag.tsx and node-actions.tsx both had the identical bug:
`node.nodeFlag = flagValue` — mutating a prop directly (not allowed),
AND assigning the OLD value (flagValue, not the newly-computed val)
since it ran before setFlagValue. Redundant besides — updateNode()
right below does the real, correct update through the store. Dead,
wrong, and mutating a prop for no benefit; deleted both.
- use-throttle.ts: `useRef(Date.now())` calls an impure function
unconditionally during render. Tried lazy-init-guarding it first
(react.dev's documented exception for ref initialization), but the
compiler still flags Date.now() specifically regardless of the
guard. Switched to seeding the ref at 0 instead of a captured
mount timestamp — only changes behavior for a call within `delay`ms
of mount, and arguably more correct: the first call now fires
immediately (standard leading-edge throttle behavior) instead of
being held back by an arbitrary baseline.
- sidebar.tsx: same shape, Math.random() in useMemo(() => ..., []).
useMemo is a cache hint, not a correctness guarantee — swapped for
useState's lazy initializer, which is actually guaranteed to run once.
Four were setFormData missing from useCallback deps in
details-panel.tsx — React Compiler's dependency inference disagreed
with the manually-written array (setFormData is stable so this was
never a staleness bug, but the compiler wants it listed to preserve
memoization). Added it to each.
sketch-list.tsx: deps array used `investigation?.sketches` but the
body accesses both that and bare `investigation.sketches` post-guard —
different expression shapes broke the compiler's dependency matching.
Tracking `investigation` directly instead resolves it. Also added a
missing `key` prop on the same list's map() while in the file — found
by a separate rule (react/jsx-key), real risk of state bleeding
between list items on reorder/filter.
Three TanStack Virtual useVirtualizer() call sites (icon-picker/popup,
nodes-view, relationships-view) are a documented React Compiler
incompatibility — the library returns functions that can't be safely
memoized, and this project has the compiler enabled
(babel-plugin-react-compiler is in the Vite config). No code change
fixes this; disabled with the reasoning inline rather than pretending
otherwise.
Verified: eslint clean outside data-table.tsx, tsc 89 (unchanged),
vitest 15/15, yarn build clean.
31 findings. Overwhelming majority were the same shape: an effect
syncing local state to a prop/value whenever it changes (form fields
re-initialized from a loaded record, a draft input reset when its
source value changes, a selection index reset when the list changes).
React's documented fix for this is "adjust state during render" —
compare the incoming value against a tracked previous value, and if it
differs, call the setters directly in the render body instead of
deferring to an effect. Same visible behavior, one fewer render pass,
and it's what the effect was silently relying on causing anyway.
A few resolved by removing the state/effect entirely instead:
- three.tsx: selectedColor was just a synced copy of a value already
recomputed fresh every render from the editor — deleted the
redundant state and effect, used the derived value directly.
- merge-nodes.tsx: old/preview were pure functions of
selectedNodes + priorityIndex — replaced with useMemo.
- import-preview.tsx: a one-time derivation gated by an isInitialized
flag, computed from data already available at render time — no
effect needed, guarded during render instead.
- json-viewer.tsx / yaml-editor.tsx: a duplicated useResolvedTheme
that just mapped theme to 'dark'/'light' — pure computation, no
state needed at all.
- custom-types route: a second effect existed only to strip spaces
from `name` after every change — moved the sanitization into the
input's onChange (and the one other assignment site) and deleted
the effect outright, rather than reacting to its own output.
use-mobile.ts and carousel.tsx were the "sync initial value, then
subscribe for future changes" pattern — kept the subscription in the
effect (that part's correct), moved the initial sync to a lazy
useState initializer (use-mobile.ts, where the value's available
synchronously) or the render-time-adjustment technique (carousel.tsx,
where `api` isn't available until later).
graph/index.tsx: the useLayoutEffect I introduced in the refs commit
for rcDepsVersion was still flagged — setState in a layout effect is
still setState in an effect. Replaced with the same render-time
comparison used everywhere else in this commit, dropping the effect
entirely.
Two left as documented eslint-disables rather than restructured:
- flows/editor.tsx: resetSimulation's calls inside the simulation
effect are a timer-driven step sequencer reaching its end state, not
derived-state-from-props — restructuring risks changing playback
timing for marginal benefit.
- tutorial-provider.tsx: the whole effect is dead in practice
(<Joyride> is never rendered, per the no-unused-vars commit) — not
worth restructuring timing on a feature nobody can see run.
Verified: eslint clean, tsc 89 (was 88 — same pre-existing
.replaceAll()/lib-target gap now hit twice instead of once, not a new
error class), vitest 15/15, yarn build clean.
10 findings, three shapes:
Six were the same pattern: `const xRef = useRef(x); xRef.current = x`
written directly in the render body, to keep a ref in sync with the
latest value for callbacks/subscriptions that need to read it without
re-subscribing (graph-main.tsx's ForceGraph2D click handler,
use-link-creation.ts, template-editor.tsx, use-confirm-dialog.tsx,
use-paste-listener.ts). Mutating a ref during render is unsafe — if
render runs without committing (Strict Mode, concurrent features), you
still perform the mutation. Moved each into useLayoutEffect (not
useEffect — this needs to happen before paint, so any interaction
right after commit sees the latest ref). use-confirm-dialog.tsx's ref
held a freshly-allocated closure every render too; wrapped that in
useCallback([]) first since it only closes over stable setState
setters, so the effect syncing it to the ref only needs to run once.
graph/index.tsx was the same shape but inside a useMemo, using the
ref mutation as a side-channel version counter for cache invalidation
(useMemo isn't supposed to have side effects, and the existing
eslint-disable-next-line on it suggests this was already known to be
irregular). Replaced with what it was actually trying to be: real
state, bumped in a useLayoutEffect keyed on the same deps — same "one
new value per deps change" behavior, done through React's actual
update path instead of a ref side-channel.
image-view-block.tsx read getComputedStyle(containerRef.current)
directly in the render body — null on first render (before mount), and
reading DOM layout during render generally isn't safe. Moved to
useState + useLayoutEffect, measuring after commit instead.
details-panel.tsx: kept as a documented eslint-disable, not fixed.
handlePropertyBlur reads a ref but only ever runs from a real
onCheckedChange handler, never during render — the lint rule's static
analysis treats any function that touches a ref as unsafe wherever
it's referenced, not just where it executes, and can't prove
otherwise from this callback shape. Restructuring around a rule
limitation isn't worth it; disabled with the reasoning inline.
Verified: eslint clean, tsc 88 (unchanged), vitest 15/15, yarn build
clean.
15 findings, two distinct root causes:
useIcon() call sites (13 of 15): useIcon returns a useCallback-memoized
function shaped like a component render function, and every call site
used it as JSX — <SourceIcon size={16} />. Since that function is
recreated whenever useIcon's deps change (type, colors, iconName...),
React saw a new component type each time and unmounted/remounted the
icon subtree on every relevant re-render. Root cause is the hook's
shape, but the minimal fix is at each call site: call it as a plain
function — SourceIcon({ size: 16 }) — instead of using it as a JSX
tag. Same output (it already returns a JSX.Element), but React never
treats it as a component to mount/unmount; it's just inline element
construction. Left table/columns.tsx's instance fixed too even though
that file has no consumers (already established dead code) — free
consistency, no reason to skip it.
new-investigation.tsx (the other 2): InvestigationForm was a component
defined inside NewInvestigation's render body — a fresh component type
every render, so any parent re-render while the create-investigation
dialog was open would unmount/remount the whole form, silently
dropping whatever the user had typed. Hoisted to module scope as a
proper component taking form/isSubmitting/onSubmit/onCancel as props.
Caught two real regressions from my own first pass before verifying:
sed replacements without line-anchoring hit more than the intended
line — `ref` got stripped from a second, unrelated Controller in
dynamic-form.tsx that actually needed it (SelectTrigger ref), and
`catch (e)` got collapsed to `catch {}` on two blocks in
use-create-on-paste.ts that still read e.message, which would have
been a ReferenceError at runtime the first time either catch fired.
Found via tsc diffing against the pre-change baseline, not by re-reading
my own edits and assuming they were right — fixed before committing.
Verified: eslint clean, tsc 88 (was 89, net improvement holds), vitest
15/15, yarn build clean.
44 findings. Most were mechanical (dead imports, unused catch bindings
converted to optional catch, unused destructured params prefixed with
_ per this repo's convention — had to add varsIgnorePattern and
destructuredArrayIgnorePattern to .eslintrc.cjs, argsIgnorePattern
alone doesn't cover `const [x, _y] = useState()` or `{ x: _y }`).
Two were real bugs, not lint noise:
- layout.worker.ts: computeDagreLayout accepted a `dagLevelDistance`
option but ignored it, hardcoding nodesep/ranksep to 5 — while the
main-thread version of the same layout (lib/utils.ts,
getDagreLayoutedElements) correctly honors it. Depending on whether
a layout ran on the worker or the main thread, the density setting
either worked or silently did nothing. Fixed the worker to match.
- investigations/$investigationId/index.tsx: a full createAnalysisMutation
(with its own onSuccess navigation and error toast) was defined and
never called — dead, superseded by the useCreateAnalysis hook used
elsewhere. Deleted rather than silenced.
One was a real gap, fixed conservatively: type-preview.tsx received
`status` and `category` props and dropped both on the floor instead of
showing them in the entity card preview. Added a minimal draft badge
and category line matching the existing preview style — not a redesign,
just displaying what was already being passed in.
Found, not fixed — three more disabled-but-built features surfaced by
this pass (same category as the dead createAnalysisMutation above, but
these are UI/product calls I'm not making unilaterally):
- tutorial-provider.tsx: full react-joyride tour wiring (steps,
callback, state) with <Joyride> never actually rendered — left as
visible warnings, not silenced, since silencing would hide that the
onboarding tour doesn't run.
- use-keyboard-events.ts: useCreateOnPaste (paste text/image to create
a node) has its import and call both commented out — the hook it
calls is dead code with zero live call sites in the app.
- map.tsx: a style-variant switcher (satellite/standard) is fully
implemented but its JSX is wrapped in a comment — setStyleVariant is
prefixed _ rather than silenced away.
Verified: eslint no-unused-vars clean outside tutorial-provider.tsx
(left intentionally), tsc 89 (was 109 — removing genuinely dead code
fixed several TS6133s that overlapped), vitest 15/15, yarn build clean.
Grab-bag of one-off rules, ~13 sites, each a genuine (if minor) issue:
- api.ts: catch (error) { throw error } — pure no-op wrapper, removed
- 4x no-unused-expressions: `cond && fn()` / ternary used as a
statement purely for its side effect — works, but not what these
operators are for. Converted to if/else.
- 2x empty interface extending its own supertype (SpinnerProps,
InputProps) — type alias instead, same shape, no vestigial interface
- resizable-container.tsx: empty catch — was silently swallowing a
bad-JSON localStorage read on purpose; added the comment saying so
instead of leaving a blank block that reads as forgotten
- 2x hasOwnProperty called directly on a plain object instead of via
Object.prototype.hasOwnProperty.call — the direct form breaks if the
object ever has a key named `hasOwnProperty` or a null prototype
- 2x stray leading `;` before a parenthesized expression, left over
from an ASI-safety pattern that doesn't apply at that position
- memoized-markdown.tsx: SyntaxHighlighter got its content via a
`children` prop instead of JSX children — same result, but the prop
form is deprecated in this React version
Verified: eslint clean for all seven rules, tsc 109 (unchanged),
vitest 15/15, yarn build clean.
no-unescaped-entities (29): raw quotes/apostrophes in JSX text escaped
to "/'. Cosmetic, zero behavior change — plain mechanical fix.
ban-ts-comment (29 -> 0 @ts-ignore, all converted): the rule bans
@ts-ignore in favor of @ts-expect-error (fails loudly if the
suppressed error is ever fixed, instead of silently rotting) and
requires a description. For each site: temporarily disabled the
directive, ran tsc to capture the real error underneath, and wrote the
description from that — not guessed. 10 were dead (no longer suppressing
anything, code had moved on) and got deleted rather than descriptioned.
The other 19 needed a genuine description. Two recurring patterns:
most are `colors[X]`/`settings[X]`-style indexing where X is a plain
string but the target type is keyed by a narrower literal union
(ItemType, Lucide icon names, a settings schema) — real gap between
runtime data and the type, not a suspicious cast. A few are structural:
lucide-react's deep icons import ships no types, one Zustand store
indexes with a runtime string, and one navigate() call passes a
$typeId param value ('new') through a typed `to` field that only
recognizes route patterns.
Caught my own mistake mid-pass: an isolated test wrongly concluded two
directives in the investigations $type route were dead; diffing tsc
output against the pre-change baseline showed 2 new errors, meaning
they were live. Restored both with real descriptions before proceeding
— didn't trust the first result, checked it against tsc directly.
Verified: eslint 0 for both rules, tsc 109 (unchanged, only line-number
churn), vitest 15/15, yarn build clean.
52 findings, 41 in live code (11 in table/data-table.tsx — no consumers
anywhere in src, left as-is, same dead-code call as the rules-of-hooks
commit). Each fixed by understanding the actual data flow, not by
mechanically satisfying the linter:
- analysis-editor.tsx: useCallback(debounce(fn), deps) is an anti-pattern
the plugin now hard-errors on — debounce() runs fresh every render
regardless of deps, so React's memoization never actually applied.
Replaced with a ref-based timer so useCallback wraps a real inline
function.
- params-dialog.tsx, sketches/index.tsx: effects did
setX({...x, ...newStuff}) with x in scope but not in deps — adding it
naively would infinite-loop (the effect's own setState re-triggers
it). Fixed with functional updates where the setter supports it
(React state), or store.getState() to read current value without
subscribing where it doesn't (Zustand actions aren't setState).
- flows/editor.tsx: resetSimulation was recreated every render and used
in an effect's deps — same instability risk. Memoized it and moved
the declaration above its first use.
- graph-main.tsx: two callbacks depended on handleBackgroundClick (has
real, changing deps) when they actually called stableBackgroundClick
(the ref-wrapped, genuinely stable version) — dependency listed
didn't match what the closure used.
- map.tsx: JSON.stringify(processedLocations) as a dep — replaced by
memoizing processedLocations itself against its real inputs, so
downstream memos can depend on it directly.
- background/index.tsx: gapXY/offsetXY are new array literals every
render; depending on them would restart a rAF loop every render.
Kept the primitives they derive from as deps, disabled the literal
rule with an explanation (matches the pattern already used elsewhere
in this file's sibling).
- template-editor.tsx: `x || {}` produced a new object each render
whenever x was absent, invalidating a memo downstream. Wrapped in
useMemo.
- settings.tsx: ref cleanup read debounceTimers.current directly in the
effect's cleanup closure, which can point at a different value by
unmount time — captured it into a local inside the effect body first.
- Rest: genuinely missing/unnecessary deps — added or removed, verified
against each callback's actual body before touching the array.
Found and fixed one real regression from this pass in review: an
effect fix in sketches/index.tsx assumed setFilters supported
functional updates like React's setState — it's a plain Zustand
action, doesn't. Caught by tsc, fixed with store.getState() instead.
Verified: yarn build, yarn test (15/15), tsc error count 109 (was 110,
pre-existing backlog — one accidental fix along the way, not hunted).
Previous commit added known_first_party per package, but isort doesn't
resolve config per-file when given multiple package roots in one
invocation (as `make lint` does) — it resolves once, from whichever
config it finds first, and applies that to every file in the run. Only
flowsint-api's list had "app" (its own internal module name); the
other three packages' configs — which the batch run could end up
using instead — didn't, so `app.*` imports in flowsint-api files were
silently misclassified as third-party depending on invocation order.
Fix: same known_first_party list, including "app", in all four
pyproject.toml. Harmless where "app" doesn't occur; makes the batch
run's config choice irrelevant since all four now agree.
Verified: every touched file checked individually now matches what
`make lint`'s single multi-package invocation produces — the actual
failure mode from the previous commit (passes alone, fails in the
batch CI runs, or vice versa).
Without known_first_party, isort's own-package detection depends on
which files it's given: sorted alone, a file inside flowsint_enrichers
gets flowsint_enrichers imports grouped separately from flowsint_core/
flowsint_types; sorted together with the rest of the workspace (as
`make lint` and CI do), they collapse into one group. lint-staged only
ever passes the staged subset, so a commit could pass the pre-commit
hook locally and still fail `make lint` in CI — silently, since both
are "isort says it's fine," just about different file sets.
Pin the four packages as known_first_party in all four pyproject.toml
so grouping is the same regardless of invocation scope. The rest of
the diff is isort re-normalizing ~100 files to the now-consistent
order — import statements only, verified against the diff.
19 real violations, surfaced by getting eslint to actually run (it was
never installed — see the CI commit). Each is a genuine hook-order bug,
not a lint nitpick:
- enricher-item.tsx: useNodesDisplaySettings/useMemo called inside a
.map() over categories — hook count varied with list length, risking
"Rendered more/fewer hooks than expected". Extracted CategoryBadge so
each item is a real component instance.
- enricher-list-item.tsx, enricher-node.tsx, flow-sheet.tsx: useIcon
called conditionally in a ternary, same pattern in all three. Now
always called (with a resolved key); the null case is decided after
the call, not before it.
- columns.tsx: useGraphStore/useCallback called inside `header`/`cell`
functions, which are plain functions invoked by flexRender, not
components. Extracted SelectAllHeaderCell/SelectRowCell/NodeLabelCell
as real components. Also fixed a stale-closure bug caught by
exhaustive-deps in the process (useCallback with an empty dep array
that should have included row.original/toggleNodeSelection). Note:
this file and data-table.tsx have no consumers anywhere in src —
dead code today, fixed for correctness in case it's wired up later,
not because it affects production.
- __root.tsx: not a real bug — the route's component fn IS a proper
React component (the router renders it as one), just anonymous, so
eslint's name heuristic couldn't tell. Named it RootComponent.
Verified: `yarn build` succeeds, vitest 15/15, tsc error count
unchanged (110, pre-existing backlog — not this branch's job).
Each is a real defect, not style — flake8 just never ran before to
catch them:
- logger.py: bare `except:` around a queue drain swallowed everything
including KeyboardInterrupt/SystemExit instead of just queue.Empty
- dockertool.py: bare `except:` narrowed to Exception
- permissions.py: duplicate `HTTPException` import, dead code
- test_registry.py: `test_get_by_lower_key` was defined twice — the
second definition silently shadowed the first, so only one ever ran
- to_ports.py / test_investigation_repository.py: assigned-but-unused
local variables, dead code
- to_whois.py / to_infos.py: documented pyflakes false positive
(OutputType is a class attr, resolves fine at def-time) — noqa'd
with an explanation instead of restructuring working code
- E712 (`== True`/`== False` → `is True`/`is False`) across 7 test
files, F541 (f-strings with no placeholders) across 5 enrichers,
E402 (import order) noqa'd in 2 files where the order is deliberate
(env vars set before import; documented circular-import workaround)
flake8/mypy/eslint were configured (or, for eslint, declared) but no CI
job and no pre-commit hook ever executed them — black/isort/flake8 had
no line-length agreement either (flake8 defaulted to 79, black to 88).
- .flake8: align max-line-length with black, ignore black-conflicting
codes, carve out the one file with an intentional wildcard-export API
- Makefile: `make lint` (black/isort/flake8, blocking — repo is clean),
`make typecheck` (mypy, ratcheted to files the branch touched — ~2200
pre-existing errors workspace-wide, not fixed blindly in one pass)
- .github/workflows/lint.yml: runs the above in CI, plus the frontend
equivalents (prettier/eslint blocking where clean, eslint/tsc
ratcheted where not)
- .github/workflows/tests.yml: add the vitest job that never existed
- flowsint-app/.eslintrc.cjs: parser was declared but never installed
(yarn lint threw immediately) — install @typescript-eslint/*, add
react-hooks/recommended; no-explicit-any set to warn (real backlog,
not fixed blindly)
- lint-staged + husky pre-commit: mirrors the CI checks that are
actually clean today (black/isort/flake8, prettier). eslint is left
out of pre-commit on purpose — its backlog would block unrelated
commits; it stays a CI-only ratchet until paid down.
workflow_dispatch button to bump version (patch/minor/major/custom),
update CHANGELOG.md, commit, tag and push to main via standard-version
— reusing the existing yarn release / sync-versions.js / .versionrc
setup instead of duplicating bump logic in YAML. The tag push then
triggers images.yml as-is.
Requires a GH_PAT secret: pushes made with the default GITHUB_TOKEN
don't trigger other workflows (GitHub anti-loop policy), so a plain
checkout token would silently skip the images.yml build.
sync-versions.js now also bumps the 4 workspace pyproject.toml files
(flowsint-api/core/enrichers/types) — previously only root
pyproject.toml and flowsint-app/package.json were kept in sync.
standard-version added as an explicit devDependency (was only
resolved transitively via yarn.lock before).
flowsint-app/package.json was stuck at 1.0.0 and root pyproject.toml
at 1.2.8, both behind the actual v1.2.11 tag. standard-version reads
flowsint-app/package.json as its source of truth, so the next release
would have tagged v1.0.1 — a semver regression breaking the Docker
image tags in images.yml.
Container couldn't open /etc/nginx/nginx.conf (permission denied) on
SELinux-enforcing hosts (e.g. Fedora Kinoite) because the bind-mounted
file kept its host label. Add :Z to relabel it for container-only
access; no-op on hosts without SELinux.
Fixes#207
Two enrichers silently return empty results instead of failing loudly.
1. domain_to_dns
dnsx >= 1.2 treats -d as "bruteforce this domain" and exits with
"missing wordlist(w) flag required with domain(d) input" unless -w is
also given, so `dnsx -d example.com -a -aaaa -json -silent` never
produces output. The enricher catches nothing (the tool exits 0) and
returns [], while Celery still reports the task as succeeded.
Feed the single target on stdin instead, which is the supported
single-target invocation. DockerTool.launch() gains an optional
entrypoint override so a shell can be put in front of the image
entrypoint; the domain is passed through shlex.quote().
2. ip_to_fraudscore
Scamalytics assigns each account a numbered API node (api11, api12,
...) and the host was hardcoded to api12, which returns 404 for every
account provisioned on a different node. The host is now read from a
SCAMLYTICS_API_HOST secret/env var, defaulting to the previous api12
so existing deployments are unaffected.
Verified against dnsx v1.2.2 and a live Scamalytics v3 account.
ctx.clip() without save/restore was permanently mutating the clipping
region across all subsequent canvas draw calls, causing uncontrollable
scaling on nodes and background when a node had an imageUrl.
dextmorgn
Mohammad Rezwan KhanandGitHub
Yvanss971
Ghraven
Topman3579