[shaktimaan00]

Closes #9964.

Went through every // TODO: Review and remove this eslint-disable once dependencies are validated left by the exhaustive-deps rollout and resolved each one:

• authContext.tsx: refreshUser now uses a functional setStatus update instead of reading status from its closure, so its deps array can stay [] without lying about it. The mount effect now depends on refreshUser directly, which is safe since its identity is now actually stable.
• configProvider.tsx: kept user as a dependency, replaced the TODO with a comment explaining why — config can differ per user even though getConfig() doesn't read user itself.
• metadata.tsx: pulled m[1] into a local variable so the dependency array isn't a complex expression eslint can't check.
• fileRenderers/simple.tsx: added the missing presign dependency.
• fileRenderers/useMarkdownProcessor.tsx: added the missing repoId, refId, path, presign dependencies — the effect actually uses all of them to build the markdown pipeline.
• fileRenderers/data.tsx: kept the disable (with a comment this time) — the SQL query is only meant to re-run on Run/submit or when the file changes, not on every keystroke in the editor.

Verified with npx eslint src (clean, no remaining exhaustive-deps violations in these files) and npx vitest run (all 72 existing tests pass). Also ran vite build to make sure nothing broke at build time.

[CLAassistant]

All committers have signed the CLA.

[yigitcan-ozturk]

Hi Anurag,

Thanks for taking this on and reviewing all the existing exhaustive-deps TODOs.

I reviewed the changes in PR #10607. The dependency fixes and the explanations for the intentionally retained exceptions look reasonable overall.

One area worth validating before merge is the SQL editor behavior in fileRenderers/data.tsx.

Since sql is intentionally excluded from the effect dependencies, could we confirm with a targeted test that editing a query and then pressing Run always executes the latest SQL text, rather than a stale closure value?

This would help distinguish the intended run-on-submit behavior from a potential stale-state issue.

Thanks for the cleanup.

Best regards,
Yiğitcan Öztürk
PAMILANGA Ltd.

[shaktimaan00]

@yigitcan-ozturk — thanks for the review, and for spotting that sql needed explicit coverage rather than just a comment.

I added the targeted test you asked for in commit 4fd581d: webui/src/pages/repositories/repository/fileRenderers/data.test.tsx. It renders DuckDBRenderer with runDuckDBQuery mocked and asserts:

• mount runs the initial READ_PARQUET(...) query;
• edit then press Execute — the last runDuckDBQuery call receives the edited text (SELECT 42), not the mount-time query — and editing alone does not re-run the query (that is the whole point of keeping sql out of the deps, otherwise every keystroke would hit DuckDB);
• edit then Ctrl+Enter (onRun in the editor) behaves the same way.

To check the test actually distinguishes the intended run-on-submit behaviour from the stale-closure risk you described, I temporarily made the effect read the SQL captured at mount (useRef(sql).current): both "edited" cases fail with the mount-time query (Expected "SELECT 42" / Received "SELECT * FROM READ_PARQUET(...) LIMIT 20"), and pass again with the current code. So a stale closure would be caught.

sql remains intentionally excluded from the effect deps, and the existing comment above the effect is unchanged.

Local run: npm test → 7 files / 75 tests pass, npm run lint clean, prettier --check clean. Happy to adjust if you'd rather the assertion live somewhere else (e.g. a hook-level test instead of the component).