From 7f2df3b1a7874c25bb6667aa8840854fb0b07787 Mon Sep 17 00:00:00 2001 From: Rodolfo Hansen Date: Fri, 4 Sep 2026 01:36:15 +0200 Subject: [PATCH] Clean optics examples and eliminate diagram overlaps - Give Prism.andThen, PartialLens.each and Plated.everywhere fluent APIs; visible examples now read like the eo cookbook instead of global compose/helper calls. - Correct Plated.everywhere to bottom-up order and make traversal preview search for the first actual focus. - Redraw the koan and all example SVGs with a wide gutter, marker-ended arrows, fill-only highlights and no border markers/dashed text overlays; render-checked at desktop and mobile widths. - Generalize the workflow diagram prompt to any screenshot runtime and encode the no-overlap layout rules in LESSONS.md. --- .claude/skills/refactoring-entry/LESSONS.md | 15 +++- .claude/skills/refactoring-entry/workflow.js | 4 +- .../01-rename-var/After.hs | 4 +- .../01-rename-var/After.scala | 2 +- .../01-rename-var/diagram.svg | 61 +++++++--------- .../02-rename-tree/After.hs | 4 +- .../02-rename-tree/After.scala | 4 +- .../02-rename-tree/diagram.svg | 70 ++++++++----------- .../03-bump-oks/After.hs | 10 +-- .../03-bump-oks/After.scala | 5 +- .../03-bump-oks/diagram.svg | 52 +++++++------- .../diagrams/koan.svg | 60 +++++++--------- .../shared/Optics.hs | 42 +++++------ .../shared/Optics.scala | 32 +++++---- 14 files changed, 177 insertions(+), 188 deletions(-) diff --git a/.claude/skills/refactoring-entry/LESSONS.md b/.claude/skills/refactoring-entry/LESSONS.md index 68c4200..8676b5c 100644 --- a/.claude/skills/refactoring-entry/LESSONS.md +++ b/.claude/skills/refactoring-entry/LESSONS.md @@ -143,8 +143,21 @@ brief.md or workflow.js and deleted here. Keep the calibration table current. hand-defined `everywhere` that used to sit in an "across a whole tree" example's After and buries the point (the optic is the reusable bit, not the walk). If the example needs shared walk machinery, add `Plated`/`everywhere` to `shared/` (a `trait`/`class` with a `descend` - instance per type; `everywhere f s = descend (everywhere f) (f s)`) and declare the type's + instance per type; `everywhere f s = f (descend (everywhere f) s)`) and declare the type's `Plated` instance in the example — the walk comes from the library, the example only says which fields recurse. Same for any helper (a fold over the tree, a traversal builder): it belongs in `shared/` or a library, not re-derived in the example. → folded into SKILL.md §3 and brief.md ("Never hand-write helpers"). + + +## 2026-09-26 — 02 clean examples and diagram overlap review + +- **Examples.** Keep domain optics named and compositional (`varP.andThen(nameL)`, + `.each`, `Plated.everywhere`); do not expose global `compose` calls or intermediate + values that the fluent API can express. Corrected `Plated.everywhere` to bottom-up: + transform children through `descend`, then apply the node rewrite. +- **Diagrams.** Numbered circles on panel borders, arrows crossing boxes and dashed + highlights through text produced visible overlap artifacts. Use a wide center gutter, + marker-ended arrows entirely inside that gutter, and low-alpha highlight fills with + no stroke. Render the actual SVG at desktop and mobile widths before assembly. + → folded into workflow.js's diagram prompt. diff --git a/.claude/skills/refactoring-entry/workflow.js b/.claude/skills/refactoring-entry/workflow.js index 88a2ce2..b6a2a87 100644 --- a/.claude/skills/refactoring-entry/workflow.js +++ b/.claude/skills/refactoring-entry/workflow.js @@ -175,8 +175,8 @@ Re-run every spec in both languages and sh ${EX}/run.sh; re-run the reviewer's m You are the DIAGRAM agent. Produce small inline-SVG diagrams for the page /refactorings/${A.slug}/, saved under ${EX}/. Read ${REPO}/pages/refactorings/extract-method/diagrams/koan.svg and one of its NN-*/diagram.svg first and match their visual language exactly (same stroke widths, fonts, arrow style, dashed region convention). - diagrams/koan.svg — the "to and from" pair for this refactoring: left = before-shape, right = after-shape, top arrow labelled with the move pointing right, bottom arrow labelled with the inverse pointing left. Use the terms from ${SCRATCH}/research.md's "To and from" section. -- NN-/diagram.svg for each example directory present (read Before/After in that dir first): left = Before with the affected region drawn as a dashed inner rectangle; right = After with the new structure and the relationship (call, instance, parameter) named on the arrow. Use the real names from the code. -Rules: viewBox-based, no fixed width/height; all strokes and text use currentColor so it works in light and dark themes; fills only rgba with low alpha or none; font-family: inherit; font-size 12–14 viewBox units; for accessibility; role="img"; no external resources, no scripts, no site CSS classes; each file under 6 KB; NO {{ or {% sequences. Render-check each SVG with the chrome-devtools MCP tools (ToolSearch for mcp__chrome-devtools__new_page / take_screenshot; file:// URLs work) at 1000px and 360px wide and fix overlaps or clipped text. Return the list of files written and notes.`, +- NN-<ex>/diagram.svg for each example directory present (read Before/After in that dir first): left = Before, right = After, with the relationship (call, instance, parameter) named in a wide center gutter. Highlight affected regions with a low-alpha fill only; do not put dashed strokes through text, numbered circles on panel borders, or arrows across boxes. Use the real names from the code. +Rules: viewBox-based, no fixed width/height; all strokes and text use currentColor so it works in light and dark themes; fills only rgba with low alpha or none; font-family: inherit; font-size 12–14 viewBox units; <title> for accessibility; role="img"; no external resources, no scripts, no site CSS classes; each file under 6 KB; NO {{ or {% sequences. Render-check the real SVG at 1000px and 360px wide with whatever screenshot facility the current agent runtime provides (headless Chrome/Firefox, browser automation, or an image-rendering tool); inspect the images and fix every overlap or clipped label before returning. Return the list of files written and notes.`, { label: 'diagrams', phase: 'Diagrams', schema: DIAGRAM_SCHEMA }) } return { examples, verdict, diagrams } diff --git a/pages/refactorings/replace-mutable-fields-with-lenses/01-rename-var/After.hs b/pages/refactorings/replace-mutable-fields-with-lenses/01-rename-var/After.hs index f6aa34c..bd5e5b7 100644 --- a/pages/refactorings/replace-mutable-fields-with-lenses/01-rename-var/After.hs +++ b/pages/refactorings/replace-mutable-fields-with-lenses/01-rename-var/After.hs @@ -3,7 +3,7 @@ module After where import Data.Char (toUpper) -import Optics (Lens(..), Prism(..), PartialLens(..), composeO, over) +import Optics (Lens(..), Prism(..), PartialLens(..), andThen, over) data Var = Var { vName :: String, vRef :: Int } deriving (Eq, Show) @@ -20,7 +20,7 @@ nameL :: Lens Var String nameL = Lens { view = vName, set = \(v, n) -> v { vName = n } } varName :: PartialLens Expr String -varName = composeO varP nameL +varName = varP `andThen` nameL upperVarName :: Expr -> Expr upperVarName = over varName (map toUpper) diff --git a/pages/refactorings/replace-mutable-fields-with-lenses/01-rename-var/After.scala b/pages/refactorings/replace-mutable-fields-with-lenses/01-rename-var/After.scala index ec14d6d..83f493b 100644 --- a/pages/refactorings/replace-mutable-fields-with-lenses/01-rename-var/After.scala +++ b/pages/refactorings/replace-mutable-fields-with-lenses/01-rename-var/After.scala @@ -18,6 +18,6 @@ object After: ) val nameL: Lens[Var, String] = Lens[Var, String](_.name, (v, n) => v.copy(name = n)) - val varName: PartialLens[Expr, String] = compose(varP, nameL) + val varName: PartialLens[Expr, String] = varP.andThen(nameL) def upperVarName(e: Expr): Expr = over(varName, _.toUpperCase)(e) diff --git a/pages/refactorings/replace-mutable-fields-with-lenses/01-rename-var/diagram.svg b/pages/refactorings/replace-mutable-fields-with-lenses/01-rename-var/diagram.svg index d501ace..1eb273b 100644 --- a/pages/refactorings/replace-mutable-fields-with-lenses/01-rename-var/diagram.svg +++ b/pages/refactorings/replace-mutable-fields-with-lenses/01-rename-var/diagram.svg @@ -1,41 +1,34 @@ -<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 780 200" role="img" aria-labelledby="d1-title" style="width:100%;height:auto;font-family:inherit;font-variant-ligatures:none;font-feature-settings:'calt' 0"> - <title id="d1-title">Replacing a hand-written match with a composed prism and lens. Before: upperVarName(e) matches Var(n, r), rebuilds it, and passes every other shape through; the dotted region is the match and rebuild. After: varName = prism(varP).andThen(lens(nameL)) — the prism decides whether the value is a Var, the lens edits its name, and one over(varName, upper) applies the rewrite; the miss passes through untouched. - - before - after + + Before, upperVarName matches EVar and rebuilds its name. After, varP is composed with nameL, then over applies uppercase; misses pass through. + + + + + + + before + after - - + + - - - + + + upperVarName(e) = e match - case Var(n, r) => Var(n.toUpper, r) - case other => other - match + rebuild - pass-through - varName = varP .andThen nameL - over(varName, _.toUpperCase): - prism decides the hit (Var) - lens edits the name - miss passes through - - - - - - - 1 - 1 - - - - - - - + EVar(v) → EVar(v.copy(name = upper)) + other → other + navigation and edit are one match + each use repeats the path + varName = varP.andThen(nameL) + varP: choose the EVar branch + nameL: focus the name field + over(varName, upper) + misses pass through unchanged + + + compose the path diff --git a/pages/refactorings/replace-mutable-fields-with-lenses/02-rename-tree/After.hs b/pages/refactorings/replace-mutable-fields-with-lenses/02-rename-tree/After.hs index eda071a..6fc9482 100644 --- a/pages/refactorings/replace-mutable-fields-with-lenses/02-rename-tree/After.hs +++ b/pages/refactorings/replace-mutable-fields-with-lenses/02-rename-tree/After.hs @@ -5,7 +5,7 @@ module After where import Data.Char (toUpper) import Optics ( Lens(..), Prism(..), PartialLens(..), Plated(..) - , composeO, over, everywhere + , andThen, over, everywhere ) data Var = Var { vName :: String, vRef :: Int } @@ -28,7 +28,7 @@ nameL :: Lens Var String nameL = Lens { view = vName, set = \(v, n) -> v { vName = n } } varName :: PartialLens Expr String -varName = composeO varP nameL +varName = varP `andThen` nameL renameAll :: Expr -> Expr renameAll = everywhere (over varName (map toUpper)) diff --git a/pages/refactorings/replace-mutable-fields-with-lenses/02-rename-tree/After.scala b/pages/refactorings/replace-mutable-fields-with-lenses/02-rename-tree/After.scala index 89ac098..9c7ace5 100644 --- a/pages/refactorings/replace-mutable-fields-with-lenses/02-rename-tree/After.scala +++ b/pages/refactorings/replace-mutable-fields-with-lenses/02-rename-tree/After.scala @@ -24,7 +24,7 @@ object After: ) val nameL: Lens[Var, String] = Lens[Var, String](_.name, (v, n) => v.copy(name = n)) - val varName: PartialLens[Expr, String] = compose(varP, nameL) + val varName: PartialLens[Expr, String] = varP.andThen(nameL) def renameAll(e: Expr): Expr = - summon[Plated[Expr]].everywhere(over(varName, _.toUpperCase))(e) + Plated.everywhere(over(varName, _.toUpperCase))(e) diff --git a/pages/refactorings/replace-mutable-fields-with-lenses/02-rename-tree/diagram.svg b/pages/refactorings/replace-mutable-fields-with-lenses/02-rename-tree/diagram.svg index 196eeb9..c10a46b 100644 --- a/pages/refactorings/replace-mutable-fields-with-lenses/02-rename-tree/diagram.svg +++ b/pages/refactorings/replace-mutable-fields-with-lenses/02-rename-tree/diagram.svg @@ -1,46 +1,36 @@ - - Replacing a recursive match with one optic reused at every node. Before: renameAll recurses over the tree and matches+rebuilds Var at each node by hand. After: everywhere(over(varName, upper)) — the same varName optic from example 1 applied at every node, bottom-up; the walk decides where, the optic decides how. - - before - after + + Before, renameAll owns both the recursive walk and the name edit. After, a Plated instance defines which fields recurse, Plated.everywhere supplies the walk, and varName supplies the edit. + + + + + + + before + after - - + + - - - + + + - renameAll(e) = - e match - EVar(v) => EVar(v.copy(name = upper)) - EApp(f, x) => EApp(renameAll(f), - renameAll(x)) - ELam(b, bd) => ELam(b, renameAll(bd)) - the rebuild appears at - every depth - everywhere(over(varName, upper)) - walk decides where: - every node, bottom-up - varName decides how: - prism then lens, from ex. 1 - one optic, reused - at every depth - - - - - - - 1 - 1 - - - - - - - + renameAll(e) = e match + EVar(v) → edit name + EApp(a,b) → recurse a, recurse b + ELam(_,b) → recurse b + one method owns walk + edit + nesting obscures the business rule + given Plated[Expr]: descend only + Plated.everywhere supplies the walk + varName supplies the focus + upper supplies the edit + three independent, reusable parts + the example declares only recursion + + + separate walk and edit diff --git a/pages/refactorings/replace-mutable-fields-with-lenses/03-bump-oks/After.hs b/pages/refactorings/replace-mutable-fields-with-lenses/03-bump-oks/After.hs index a0517d4..f097762 100644 --- a/pages/refactorings/replace-mutable-fields-with-lenses/03-bump-oks/After.hs +++ b/pages/refactorings/replace-mutable-fields-with-lenses/03-bump-oks/After.hs @@ -2,7 +2,10 @@ -- reach every element, match the succeeded branch, edit its value. module After where -import Optics (Lens(..), Prism(..), PartialLens(..), composeO, each, over) +import Optics + ( Lens(..), Prism(..), PartialLens(..) + , andThen, each, over + ) data Ok = Ok { okValue :: Int } deriving (Eq, Show) @@ -18,11 +21,8 @@ succeededP = Prism valueL :: Lens Ok Int valueL = Lens { view = okValue, set = \(ok, v) -> ok { okValue = v } } -okVal :: PartialLens Result Int -okVal = composeO succeededP valueL - eachSucceeded :: PartialLens [Result] Int -eachSucceeded = each okVal +eachSucceeded = each (succeededP `andThen` valueL) bumpSucceeded :: [Result] -> [Result] bumpSucceeded = over eachSucceeded (+ 1) diff --git a/pages/refactorings/replace-mutable-fields-with-lenses/03-bump-oks/After.scala b/pages/refactorings/replace-mutable-fields-with-lenses/03-bump-oks/After.scala index 1159d9b..cc11b19 100644 --- a/pages/refactorings/replace-mutable-fields-with-lenses/03-bump-oks/After.scala +++ b/pages/refactorings/replace-mutable-fields-with-lenses/03-bump-oks/After.scala @@ -17,9 +17,8 @@ object After: ) val valueL: Lens[Ok, Int] = Lens[Ok, Int](_.value, (ok, v) => ok.copy(value = v)) - val okVal: PartialLens[Result, Int] = compose(succeededP, valueL) - - val eachSucceeded: PartialLens[List[Result], Int] = each(okVal) + val eachSucceeded: PartialLens[List[Result], Int] = + succeededP.andThen(valueL).each def bumpSucceeded(xs: List[Result]): List[Result] = over(eachSucceeded, _ + 1)(xs) diff --git a/pages/refactorings/replace-mutable-fields-with-lenses/03-bump-oks/diagram.svg b/pages/refactorings/replace-mutable-fields-with-lenses/03-bump-oks/diagram.svg index 217a73f..b69eb2d 100644 --- a/pages/refactorings/replace-mutable-fields-with-lenses/03-bump-oks/diagram.svg +++ b/pages/refactorings/replace-mutable-fields-with-lenses/03-bump-oks/diagram.svg @@ -1,32 +1,34 @@ - - Replacing a map-with-match with a composed traversal, prism and lens. Before: bumpSucceeded maps with a branch test and rebuild in the same step. After: each .andThen succeeded .andThen value — the traversal reaches every element, the prism selects the succeeded branch, the lens edits its value; failures pass through untouched. - - before - after + + Before, bumpSucceeded combines the list walk, branch selection and value edit in one match. After, each, succeededP and valueL compose, then over adds one. + + + + + + + before + after - - + + - - + + + - bumpSucceeded(xs) = - xs.map { case Succeeded(ok) => - Succeeded(ok.copy(value = ok.value + 1)) - case f @ Failed(_) => f } - branch test and rebuild - in the same step - each .andThen succeeded - .andThen value - every element, - succeeded branch, - edit its value - failures pass through - - - traversal over the list - composed optic + xs.map { result → result match + Succeeded(ok) → bump ok.value + Failed(msg) → keep unchanged + walk + branch + field edit + are one nested operation + succeededP.andThen(valueL).each + each: every list element + prism: success · lens: value + over(eachSucceeded, _ + 1) + failed values pass through + + compose three steps diff --git a/pages/refactorings/replace-mutable-fields-with-lenses/diagrams/koan.svg b/pages/refactorings/replace-mutable-fields-with-lenses/diagrams/koan.svg index 77bf53c..2166bee 100644 --- a/pages/refactorings/replace-mutable-fields-with-lenses/diagrams/koan.svg +++ b/pages/refactorings/replace-mutable-fields-with-lenses/diagrams/koan.svg @@ -1,38 +1,32 @@ - - Replace mutable fields with lenses / replace lenses with mutable fields. Left: a record field read and written directly, spelled out at every call site. Right: the same field as a lens value packaging get :: s -> a and set :: a -> s -> s, so every read is get, every write is set or modify. The arrow to the right is replace (package get and set as one composable value); the arrow to the left is inline (drop the lens and access the field directly). The three lens laws — get (set v s) = v, set (get s) s = s, set v2 (set v1 s) = set v2 s — are the equation the move is checked against. - - before - after + + One equation read in two directions. Before, navigation to a field and the edit are repeated together. After, the path is a composed optic and the edit is a separate function. Replace packages the path; inline writes the access directly. + + + + + + + before + after - - + + - - field balance - balanceLens = - Lens { get, set } + + reach the field + match / copy / recurse + then change it + business rule mixed + with navigation + path = prism + .andThen(lens).each + over(path, change) + navigation and rule + are separate values - - read: a.balance - write: copy with balance - every call site spells the - field out again - read: get balanceLens - write: set / modify - lens composes along paths - - - - - - - - - - - replace (package get + set into a lens) - inline (drop the lens, access the field) - - get (set v s) = v · set (get s) s = s · set v₂ (set v₁ s) = set v₂ s + + replace: package the path + + inline: write the access directly diff --git a/pages/refactorings/replace-mutable-fields-with-lenses/shared/Optics.hs b/pages/refactorings/replace-mutable-fields-with-lenses/shared/Optics.hs index 809409f..5ddb40a 100644 --- a/pages/refactorings/replace-mutable-fields-with-lenses/shared/Optics.hs +++ b/pages/refactorings/replace-mutable-fields-with-lenses/shared/Optics.hs @@ -1,9 +1,9 @@ --- Shared optic building blocks for the examples on this page: a Lens, --- a Prism, a composed PartialLens, and a traversal `each`. Compiled by --- run.sh but not shown on the page, so each example is the move itself. -module Optics (Lens(..), Prism(..), PartialLens(..), Plated(..), everywhere, composeO, each, over, modify) where - -import Data.Maybe (listToMaybe) +-- Shared optic building blocks for the examples on this page. Compiled +-- by run.sh but not shown, so each example presents only the move. +module Optics + ( Lens(..), Prism(..), PartialLens(..), Plated(..) + , andThen, each, over, modify, everywhere + ) where data Lens s a = Lens { view :: s -> a, set :: (s, a) -> s } @@ -15,35 +15,31 @@ data Prism s a = Prism , review :: a -> s } --- A prism followed by a lens: hit -> edit the field; miss -> pass through. data PartialLens s a = PartialLens { plPreview :: s -> Maybe a , plModify :: (a -> a) -> s -> s } -composeO :: Prism s m -> Lens m a -> PartialLens s a -composeO p l = PartialLens - ( \s -> view l <$> preview p s - ) ( \f s -> case preview p s of - Nothing -> s - Just m -> review p (modify l f m) - ) +andThen :: Prism s m -> Lens m a -> PartialLens s a +andThen p l = PartialLens + (\s -> view l <$> preview p s) + (\f s -> case preview p s of + Nothing -> s + Just m -> review p (modify l f m)) --- A traversal: reach every element, apply the partial lens to each. each :: PartialLens s a -> PartialLens [s] a -each pl = PartialLens - (\xs -> listToMaybe xs >>= plPreview pl) - (\f xs -> map (plModify pl f) xs) +each pl = PartialLens (firstFocus . map (plPreview pl)) + (\f -> map (plModify pl f)) + where + firstFocus [] = Nothing + firstFocus (Just a : _) = Just a + firstFocus (Nothing : xs) = firstFocus xs over :: PartialLens s a -> (a -> a) -> s -> s over pl = plModify pl --- Plated: the recursion of a recursive type, as a value. An instance --- says which fields are the sub-terms (the "plate"); `everywhere` --- then applies a rewrite at every node, bottom-up. class Plated s where descend :: (s -> s) -> s -> s everywhere :: Plated s => (s -> s) -> s -> s -everywhere f s = descend (everywhere f) (f s) - +everywhere f s = f (descend (everywhere f) s) diff --git a/pages/refactorings/replace-mutable-fields-with-lenses/shared/Optics.scala b/pages/refactorings/replace-mutable-fields-with-lenses/shared/Optics.scala index 14084d9..ed3359d 100644 --- a/pages/refactorings/replace-mutable-fields-with-lenses/shared/Optics.scala +++ b/pages/refactorings/replace-mutable-fields-with-lenses/shared/Optics.scala @@ -1,6 +1,5 @@ -// Shared optic building blocks for the examples on this page: a Lens, -// a Prism, a composed PartialLens, and a traversal `each`. Compiled by -// run.sh but not shown on the page, so each example is the move itself. +// Shared optic building blocks for the examples on this page. Compiled +// by run.sh but not shown, so each example presents only the move. object Optics: case class Lens[S, A](view: S => A, set: (S, A) => S): def modify(f: A => A): S => S = s => set(s, f(view(s))) @@ -8,35 +7,38 @@ object Optics: case class Prism[S, A](preview: S => Option[A], review: A => S): def modify(f: A => A): S => S = s => preview(s).map(a => review(f(a))).getOrElse(s) + def andThen[B](inner: Lens[A, B]): PartialLens[S, B] = + compose(this, inner) - // A prism followed by a lens: hit -> edit the field; miss -> pass through. case class PartialLens[S, A](preview: S => Option[A], - modify: (A => A) => S => S) + modify: (A => A) => S => S): + def each: PartialLens[List[S], A] = Optics.each(this) - def compose[S, M, A](p: Prism[S, M], l: Lens[M, A]): PartialLens[S, A] = + def compose[S, M, A](p: Prism[S, M], + l: Lens[M, A]): PartialLens[S, A] = PartialLens( preview = s => p.preview(s).map(l.view), - modify = f => s => p.preview(s) match + modify = f => s => p.preview(s) match case Some(m) => p.review(l.modify(f)(m)) case None => s, ) - // A traversal: reach every element, apply the partial lens to each. def each[S, A](pl: PartialLens[S, A]): PartialLens[List[S], A] = PartialLens( - preview = _.headOption.flatMap(pl.preview), - modify = f => _.map(pl.modify(f)), + preview = _.iterator.map(pl.preview).collectFirst { + case Some(a) => a + }, + modify = f => _.map(pl.modify(f)), ) def over[S, A](pl: PartialLens[S, A], f: A => A): S => S = pl.modify(f) - // Plated: the recursion of a recursive type, as a value. An instance - // says which fields are the sub-terms (the "plate"); `everywhere` - // then applies a rewrite at every node, bottom-up, without the type - // knowing how to walk itself. trait Plated[S]: def descend(f: S => S)(s: S): S def everywhere(f: S => S): S => S = - s => descend(everywhere(f))(f(s)) + s => f(descend(everywhere(f))(s)) + object Plated: + def everywhere[S](f: S => S)(using p: Plated[S]): S => S = + p.everywhere(f)