Code Review

Good, focused refactor — the mechanics are sound and the diff is net negative. A few things worth addressing before merge:

1. Duplicate DAE imports (9 files) — likely -Wunused-imports warnings

Nine files add import Deriving.Aeson.Stock qualified as DAE while keeping the old import Deriving.Aeson qualified as DAE:

src/Models/Apis/Anomalies.hs
src/Models/Apis/Issues.hs
src/Models/Apis/LogQueries.hs
src/Models/Projects/Dashboards.hs
src/Pages/LogExplorer/Log.hs
src/Pkg/SchemaLearning/Catalog.hs
src/Web/ApiHandlers.hs
src/Web/ApiTypes.hs
src/Web/Auth.hs

If the only remaining use of DAE.* in these files is now DAE.Snake (resolves to Deriving.Aeson.Stock), the old Deriving.Aeson import is dead and GHC will warn. The other nine files in this PR correctly remove the old import at the same time — apply the same treatment here.

Dashboards.hs is the worst case: it now has three Deriving.Aeson* imports (DAE → Aeson, DAE → Stock, DAES → Stock). The two Stock imports should be unified into one alias.

2. Exporting bare head from Data.Effectful.Wreq

-  W.head_,
+  head,

Network.Wreq uses head_ for exactly this reason — a bare head in a module's export list is a shadowing hazard for any importer that doesn't explicitly hide it. Since makeEffect derives the name from the constructor, the fix is to keep (or rename) the HTTP constructor as Head_ so the generated wrapper is head_, matching the Wreq convention. Relude hiding (head) at the definition site is fine but doesn't protect downstream callers.

3. ghostButton_ is added but never called in this PR

ghostButton_ is exported from Pages.Components but has no call sites in the diff. If it's for an immediate follow-up, note it; otherwise drop it — the existing button_ [class_ "btn btn-ghost btn-sm", ...] pattern is still live and would benefit from the combinator.

4. Minor: headerRow_ vs sectionHeader_ share identical base classes

headerRow_     attrs = div_ (class_ "flex items-center justify-between"      : attrs)
sectionHeader_ attrs = div_ (class_ "flex items-center justify-between mb-4" : attrs)

These differ only by mb-4. Consider making sectionHeader_ a thin alias:

sectionHeader_ attrs = headerRow_ (class_ "mb-4" : attrs)

One definition, explicit relationship, and the two combinators can't drift apart.

What's working well

• makeEffect ''LLM / makeEffect ''HTTP — textbook use of Effectful.TH; boilerplate gone cleanly.
• DAE.Snake sweep is mechanically correct; wire format is unchanged where the deriving via was already CamelToSnake + OmitNothingFields.
• lookupVec / lookupVecBy generics preserve all existing shim signatures exactly; old behaviour is reproduced by aeson's parseJSON for Text, Int, and Bool.
• withSettingsPage / withPageWrapper are the right level of abstraction; the two proof-of-concept migrations (bringS3GetH, gitSyncSettingsGetH) demonstrate the pattern clearly.
• Deferring A3 (tables) and D2 (text-display / attoparsec) is the correct call — high blast radius without HTML golden tests.

Code Review — Phase 1 Conciseness Refactor

Good, well-scoped pass overall. The workstreams are independent and the PR description is unusually thorough. A few issues to address before merging.

Bug: sectionHeader_ silently drops the mb-4 class

-- Pages/Components.hs
sectionHeader_ attrs = headerRow_ (class_ "mb-4" : attrs)
-- expands to: div_ (class_ "flex items-center justify-between" : class_ "mb-4" : attrs)

This produces two separate class attributes on the element. Per the HTML5 spec, duplicate attributes are invalid — browsers use the first value and discard the rest, so mb-4 is silently ignored. Fix by merging into one class string:

sectionHeader_ attrs = div_ (class_ "flex items-center justify-between mb-4" : attrs)

Same potential issue applies anywhere the pattern headerRow_ (class_ "..." : attrs) is used at call sites (not introduced here, but worth watching in follow-up sweeps).

Inconsistency: DAES.Snake in Widget.hs

Pkg/Components/Widget.hs uses DAES.Snake while every other file in this PR uses DAE.Snake:

-- Widget.hs (not standardised)
deriving (AE.FromJSON, AE.ToJSON) via DAES.Snake TableColumn
deriving (AE.FromJSON, AE.ToJSON) via DAES.Snake RowClickAction

The rest of the PR standardises on import Deriving.Aeson.Stock qualified as DAE. This file should follow suit: change its import alias from DAES to DAE and update the two deriving clauses, or at minimum note it as a known exception.

Redundant dual imports (minor cleanup opportunity)

Several files now carry both:

import Deriving.Aeson qualified as DAE
import Deriving.Aeson.Stock qualified as DAE  -- re-exports everything above + adds Snake

In files where the only remaining DAE.* usage is DAE.Snake (e.g. ProcessMessage.hs, Web.Routes, Web.Auth, Pkg.AI, etc.), the first import is now redundant. Deriving.Aeson.Stock re-exports the full Deriving.Aeson surface. Easy follow-up: drop the import Deriving.Aeson qualified as DAE line in those files. Not a correctness issue.

makeEffect ''HTTP and the Head → Head_ rename

The rename is the right call — makeEffect lowercases the constructor, so Head would generate head which clashes with Relude. Generating head_ from Head_ matches the conventional Wreq/Lucid spelling. Just confirm CI passes since this is a re-exported name change (the PR description says zero rg hits, which is reassuring).

lookupVec refactor (D1)

The generalisation is clean and the typed shims remain at their original signatures. One subtle note: lookupVecInt previously used Data.Scientific.toBoundedInteger for the number → Int conversion; the new path goes through AE.parseJSON @Int which uses parseIntegral internally. Behaviour is equivalent for well-formed data. The removed import Data.Scientific (toBoundedInteger) is correctly dropped.

Summary

Code Review — refactor/phase1-conciseness

Overall this is a clean, well-scoped refactor. makeEffect, DAE.Snake, and the combinator sweep all move in the right direction. A few items worth fixing before merge:

Bug: sectionHeader_ emits duplicate class attributes

-- Pages/Components.hs
headerRow_   attrs = div_ (class_ "flex items-center justify-between" : attrs)
sectionHeader_ attrs = headerRow_ (class_ "mb-4" : attrs)

sectionHeader_ [] expands to div_ [class_ "flex items-center justify-between", class_ "mb-4"].
Lucid renders both attributes literally; per the HTML spec, duplicate attributes → the first wins, so mb-4 is silently ignored.

Fix — inline the combined class string instead of delegating to headerRow_:

sectionHeader_ attrs = div_ (class_ "flex items-center justify-between mb-4" : attrs)

Inconsistent Deriving.Aeson.Stock qualifier in Widget.hs

The PR standardises on import Deriving.Aeson.Stock qualified as DAE everywhere, but Widget.hs still uses:

import Deriving.Aeson.Stock qualified as DAES   -- untouched by this PR
...
deriving (AE.FromJSON, AE.ToJSON) via DAES.Snake TableColumn   -- still DAES
deriving (AE.FromJSON, AE.ToJSON) via DAES.Snake RowClickAction

The PR touched Widget.hs to add headerRow_ but didn't normalise the DAES alias to DAE. Should be a one-line import rename + two via clause updates to match the rest of the codebase.

Dual DAE imports are now redundant in several files

Anomalies.hs and LogQueries.hs now carry:

import Deriving.Aeson qualified as DAE          -- for CustomJSON / OmitNothingFields still in use
import Deriving.Aeson.Stock qualified as DAE    -- for Snake

That's valid GHC (two modules under one qualifier), but since Deriving.Aeson.Stock re-exports the full Deriving.Aeson API, a single import Deriving.Aeson.Stock qualified as DAE covers both. Worth doing to avoid the two-import smell, at least in files like LogQueries.hs where CustomJSON is fully gone.

Minor: PR description mentions ghostButton_ but it is not in the diff

The "Workstream A" summary lists ghostButton_ as added to Pages.Components; it isn't. Fine if deferred, but should be removed from the description or tracked as a follow-up so it doesn't get lost.

Positive notes

• makeEffect ''LLM / makeEffect ''HTTP — correct use of TH; eliminates 16 boilerplate wrappers with no semantic change.
• DAE.Snake sweep across 19 files — mechanical, correct, builds green; IssueSummary's CustomJSON '[OmitNothingFields] (no field renaming) is correctly left as-is since DAE.Snake includes CamelToSnake which would change its wire format.
• lookupVec / lookupVecBy generalisation in Utils.hs is clean; the typed shims remain as one-liners so call sites are untouched.
• withPageWrapper / withSettingsPage reduce handler boilerplate nicely; the two proof-of-concept migrations (bringS3GetH, gitSyncSettingsGetH) make the pattern concrete without over-committing.
• Dropping Data.Scientific (toBoundedInteger) and the Effectful.Wreq re-exports (W.options, W.head_, W.optionsWith, W.headWith) are clean cuts — confirmed zero callers.

Code Review — Phase-1 Conciseness Refactor

Overall this is a well-scoped, clean refactor. The mechanical substitutions (Workstreams B, C, D1) are correct and the reduction from 382 lines to 173 is meaningful. A few issues to address before merging.

Bug: sectionHeader_ silently drops mb-4

-- Pages/Components.hs
sectionHeader_ attrs = headerRow_ (class_ " mb-4 " : attrs)

headerRow_ attrs = div_ (class_ " flex items-center justify-between " : attrs)

sectionHeader_ [] renders div_ [class_ "flex items-center justify-between", class_ "mb-4"] — two class attributes on the same element. Per HTML spec, duplicate attributes are invalid and browsers use only the first, so mb-4 is silently ignored. Fix by inlining the full class string instead of delegating to headerRow_:

sectionHeader_ attrs = div_ (class_ " flex items-center justify-between mb-4 " : attrs)

head in the public API of Data.Effectful.Wreq

makeEffect ''HTTP generates a function named head (from the Head constructor), which is now exported. Any caller that imports this module unqualified gets head shadowing Prelude.head/Relude.head. Network.Wreq avoids this by using head_. Consider either:

• Renaming the GADT constructor to Head_ so makeEffect generates head_ (matching Wreq's convention and the old export name), or
• Exporting it qualified-only.

The current fix (hiding (head) in the module itself) only protects the Wreq module's own body, not callers.

Inconsistent DAES alias in Widget.hs

src/Pkg/Components/Widget.hs still uses DAES.Snake for its two new derivations while every other touched file now uses DAE. This PR is the right time to normalise:

-- change existing
import Deriving.Aeson.Stock qualified as DAES
-- to
import Deriving.Aeson.Stock qualified as DAE

Potentially unused import Deriving.Aeson qualified as DAE in several files

Files where all DAE.CustomJSON derivations are replaced (Anomalies.hs, Issues.hs, LogQueries.hs, Catalog.hs, etc.) retain both:

import Deriving.Aeson qualified as DAE        -- may now be unused
import Deriving.Aeson.Stock qualified as DAE

If there are no remaining usages of DAE.OmitNothingFields, DAE.FieldLabelModifier, etc., the first import is dead. -Wall / fourmolu --check-unused-imports would surface these. Worth a pass to remove stale imports now rather than letting them accumulate.

Minor: ghostButton_ mentioned in PR description but absent from the diff

The summary lists ghostButton_ as added to Pages.Components; it's not in the export list or the diff. Either drop it from the description or add the missing combinator.

What's correct and good

• makeEffect ''LLM / makeEffect ''HTTP — generated names match the old hand-written wrappers exactly; safe drop-in replacement.
• DAE.Snake sweep — consistent, correct; Deriving.Aeson.Stock.Snake is semantically identical to CustomJSON '[OmitNothingFields, FieldLabelModifier '[CamelToSnake]].
• lookupVec / lookupVecBy — the refactor preserves exact semantics: FromJSON Text uses withText (strings only), FromJSON Int uses toBoundedInteger (integers only, no floats), FromJSON Bool uses withBool. All five shims stay as one-liners so call sites are unaffected.
• withPageWrapper / withSettingsPage — clean, the lambda-based API (\project bw -> ...) is idiomatic and composes well.
• primaryButton_, headerRow_, headerRowPad_ — correct class-prepend pattern for non-class attrs.
• Removing SnakeJSON type alias from Web.ApiTypes is the right call now that DAE.Snake is uniform.

Blockers before merge: sectionHeader_ bug, head API concern.
Nice-to-have: DAES→DAE normalisation in Widget.hs, unused import cleanup, fix PR description re: ghostButton_.

Code Review

Well-executed conciseness pass. Net −46 lines across 29 files with no semantic changes — this is exactly the kind of refactor the codebase benefits from. A few things worth discussing:

Head → Head_ constructor rename (Wreq.hs)

Renaming the GADT constructor from Head to Head_ is required for makeEffect ''HTTP to generate head_ (avoiding the Relude.head clash), and it's the right call. But the interpreter (runHTTPWreq) needs to match on Head_ now — the diff doesn't show that hunk. Since ghcid is green this must have been updated, but it'd be good to confirm it's in this PR and not silently relied on elsewhere. Worth a quick rg 'Head ' src/Data/Effectful/Wreq.hs to sanity-check no stale match remains.

Whitespace in new class_ strings

The four new component helpers have leading/trailing spaces in their class strings:

primaryButton_ attrs = button_ (class_ " btn btn-primary btn-sm " : attrs)
headerRow_     attrs = div_    (class_ " flex items-center justify-between " : attrs)

The adjacent code (e.g. modalCloseButton_) uses "btn btn-sm btn-circle …" without padding. HTML/CSS is tolerant of extra whitespace in class attributes, but the inconsistency is a minor style nit. Consider trimming.

ghostButton_ mentioned in description but absent

The PR summary lists ghostButton_ as added to Pages.Components, but it doesn't appear in the diff or the updated export list. Either an oversight in the description, or it was intentionally deferred — worth clarifying so reviewers don't go hunting for it.

lookupVecInt semantics — confirm equivalence

Old:

lookupVecInt vec idx = case vec V.!? idx of
  Just (AE.Number val) -> fromMaybe 0 $ toBoundedInteger val
  _ -> 0

New:

lookupVecInt vec idx = fromMaybe 0 (lookupVec vec idx)
-- where lookupVec uses AET.parseMaybe AE.parseJSON

parseJSON @Int internally calls parseIntegralFromNumber, which also rejects fractional values and returns Nothing for out-of-bounds inputs, so the behavior is equivalent in practice. However, the old version explicitly only matched AE.Number — a String "42" would have returned 0, whereas parseJSON @Int on a JSON string would also return Nothing → 0. Same end result. Flagging only so the author can confirm they've considered this path.

SnakeJSON alias removal (ApiTypes.hs)

Clean — DAE.Snake from Deriving.Aeson.Stock is defined as exactly CustomJSON '[OmitNothingFields, FieldLabelModifier '[CamelToSnake]], so this is a zero-risk substitution. Good catch reusing the stock alias rather than keeping the local one.

withPageWrapper / withSettingsPage

Clean abstractions. One small observation: withPageWrapper discards the Session from mkPageCtx (the _ in (_, project, bw)). If future callers need the session, the combinator would need to expose it — but for the current two migrated call sites this is fine. The lambda syntax in withSettingsPage using LambdaCase-adjacent style (\p bw -> ...) reads well.

Overall

• makeEffect ''LLM / makeEffect ''HTTP: correct use of TH — eliminates boilerplate perfectly.
• DAE.Snake sweep: straightforward and complete across 19 files.
• lookupVec/lookupVecBy generalization: elegant. The typed shims as one-liners is the right call for zero-churn callers.
• View combinators: good pattern establishment. Leaving variant-class sites alone is the right call for phase 1.

LGTM modulo the Head_ interpreter hunk confirmation and the description/ghostButton_ discrepancy.