Refactoring 2/35: Replace mutable fields with lenses

.claude/skills/refactoring-entry/LESSONS.md

@@ -75,3 +75,76 @@ brief.md or workflow.js and deleted here. Keep the calibration table current.
   after the item) attach to the *enclosing* `<ol>` at a break point and split the list. The working
   pattern is: a single `<ol>` with raw `<li id="ref-N">` items, and hand-render inline markdown to
   `<em>`/`<a>` yourself. Folded into SKILL.md §3, workflow.js research prompt, and brief.md.
+
+## 2026-09-26 — 02 Replace mutable fields with lenses
+
+- **Scala/Haskell Spec must compare *result values*, not the Before/After records.** The two sides carry
+  different record types (they must — the point is that After's type can differ). Asserting
+  `Before.x(...) ==== After.x(...)` where the tuple/record types differ makes hedgehog upcast to `Any`
+  and report structural inequality, or fail to compile without a derived `Eq`. Compare projections:
+  `(before.field, ...) ==== (after.field, ...)`. The reference extract-method avoided this by returning
+  primitives; lens examples return records, so this bites immediately. → rule in brief.
+- **A lens is a focus on one path; composition is for nested paths, not merging sibling fields.** A first
+  02 tried `compose(x, y)` on two lenses with the same source to build a pair-lens — that is not a lens
+  (no single field focus), and it does not typecheck. The textbook "compose two lenses" example is a
+  nested record: `player . x`. When the refactoring is about *every element* (all sizes in a tree), a
+  single lens does not fit — that is a traversal; the coherent example uses a total per-node lens inside
+  the recursion. → design note for future lens-like entries.
+- **A partial lens (entryFile on a sum) forces `error`/`sys.error` in the getter; a reviewer would flag
+  it and the property can only test the defined region.** Switched 03 to a total `Lens[Node, Int]` on a
+  plain record — partiality is a real pitfall to *write about*, not a good example shape.
+- **Mutation-checking discipline.** A wrong-typed mutant is not a valid test; the compose-swap mutant
+  was a type error in both languages (itself evidence the lens types are sound). Use sign flips,
+  off-by-ones, dropped cases as mutants; drop mutants that fail to compile rather than reporting a pass.
+- **Toolchain.** `scala-cli run <dir> --main-class spec` works; `docker run cp-hedgehog runghc -i<dir>
+  <dir>/Spec.hs` works. Width rule held at 72 chars for all sources. Time: examples+verify ≈ 45 min;
+  mutation+probes ≈ 15 min.
+- **Workflow-runner gap.** This environment has no Claude Code `Workflow` batch runner; executed the
+  workflow phases directly (research → citation verify → examples → mutate/probes → diagrams). Recorded
+  findings here so the skill's workflow prompt matches when the runner is available.
+
+## 2026-09-26 — 02 Replace mutable fields with lenses (post-PR review rework)
+
+- **Reviewer (kryptt) asked for the optics framing, not the lens framing.** The entry should teach
+  *optics* (lens · prism · traversal) and the "how to reach vs what to do" separation, mention that
+  optics are lawful *by construction* (not per-example law properties in the page), reuse the eo
+  cookbook recipes, and justify the inverse by decoupling not paying for itself (no cross-domain
+  boundaries). Law-solvers (cats-eo-laws, monocle-law, genvalidity-hspec-optics) belong in
+  Verification, not as hand-written law properties.
+- **Rule.** Example trios should escalate *nesting* (single node → every tree node → sparse walk over a
+  list) so the composed optic's value is visible; the setter-optic encoding (`(a -> a) -> (s -> s)`)
+  is compact but reads cryptic next to same-shaped Lens/Prism data types — prefer explicit
+  `Lens`/`Prism`/`Traversal` cases with `compose` (prism .andThen lens, each .andThen prism .andThen
+  lens) for the page.
+- **Rule (spec comparison).** Keep comparing via projected tuples/`from*` converters (Before/After
+  have distinct record types), and add a *purpose* second property (hit/miss, only-X-changed) rather
+  than also the lens laws — laws are by construction, the purpose property is the page's real check.
+- **Generators.** `Gen.unicode` yields control chars (`\NUL`) that `toUpper` leaves alone; for
+  "all names uppercased" style invariants use `Gen.alpha` (ASCII letters) or exclude non-letter
+  inputs explicitly. `Gen.list/Gen.string` argument order differs between Scala and Haskell hedgehog.
+- **Time.** Rework cost ≈ 1h (examples+specs+diagrams+page). The eo cookbook itself is the source of
+  truth for optics recipes; reference it with anchors.
+
+## 2026-09-26 — 02 shared/ setup must not appear on the page (post-PR review round 2)
+
+- **Rule (blocking).** Setup shared between examples is *accidental complexity* if shown. A lens
+  entry redefining `Lens`/`Prism`/`PartialLens` in every `After` buries the motivation in the
+  definition of the tool. Move shared machinery to `pages/refactorings/<slug>/shared/` — compiled
+  by `run.sh` (`scala-cli run "$d" --main-class spec "$SHARED"`, `runghc -i"$SHARED"`), never
+  `include_relative`'d. Executed in 02: `shared/Optics.scala|.hs` and `shared/SpecRunner.scala`;
+  the page says the shared files are hidden, each example is the move itself.
+  → folded into SKILL.md §3 and brief.md.
+- **Skill must be agent-agnostic.** It lived in `.claude/skills/` and assumed the Claude Code
+  `Workflow(...)` runner. Rewrote §2 (the workflow) to describe the phases generically — research ·
+  examples · review · diagrams — with the Claude `workflow.js`/`journal.jsonl` as one optional
+  orchestrator, and generalized the `gh`/ship steps. The `Workflow` script stays but nothing else
+  assumes Claude.
+- **Rule (blocking).** Do not hand-write intermediate helper methods in an example — like the
+  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
+  `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").

.claude/skills/refactoring-entry/SKILL.md

@@ -26,34 +26,30 @@ the steps below and the `workflow.js` prompts; if you find one that has not, fol
 1. Branch from `master`: `refactoring/<slug>`.
 2. Pick the entry: the first item in `_data/refactorings.yml` without a `slug`, unless the user
    names one. Add `slug: <slug>` to it (that is what links it on the index page).
-3. Toolchain check (the workflow agents assume these work):
+3. Toolchain check (the agents assume these work):
    - `scala-cli --version` (hedgehog `qa.hedgehog::hedgehog-core:0.14.0` + `hedgehog-runner:0.14.0`).
    - `docker image inspect cp-hedgehog` — if missing, `sh pages/refactorings/extract-method/run.sh`
-     builds it (about 2.5 minutes). GHC is not installed on the host; nix is read-only.
+     builds it (about 2.5 minutes). GHC is not installed here; nix is read-only.
 4. Write the brief to the scratchpad: copy `brief.md` from this directory, fill the `<...>` fields.
-   The workflow agents read the brief, not this file.
-
-## 2. Run the workflow (research ∥ examples → verify loop → diagrams)
-
-```
-Workflow({
-  scriptPath: ".claude/skills/refactoring-entry/workflow.js",
-  args: {
-    slug: "<slug>", name: "<Name>", number: <n>, total: 35,
-    brief: "<abs path to brief.md>", scratch: "<abs scratchpad dir>",
-    repo: "<abs repo root>",
-    seeds: ["<paper or tool that anchors the OO side>", "<the FP-side paper>", ...],
-    examples: ["<example 1 idea>", "<example 2 idea>", "<example 3 idea>"]
-  }
-})
-```
-
-- The script runs two tracks in parallel: **research → per-citation skeptics → fix-up**, and
-  **examples → reviewer (mutation + adversarial probes) → fix, up to 3 rounds → diagrams**.
-- If an agent dies (session limit, API error), relaunch with `resumeFromRunId`; finished agents replay
-  from cache. Read `journal.jsonl` before assuming a result is empty.
-- While it runs: write `pages/refactorings/<slug>.md` from the extract-method page (next step), and
-  build the site in a scratch copy with stub SVGs to catch Liquid errors early.
+   The agents that build the entry read the brief, not this file.
+
+## 2. Run the workflow (research ∥ examples → review → diagrams)
+
+This skill describes a *workflow*, not a Claude-specific runner. The phases below are mandatory;
+how you parallelise them depends on your agent runtime:
+
+- **Research** — with verified citations (one skeptic pass per reference), and
+- **Examples** — build + run the 3 Before/After/Spec pairs in both languages, then
+- **Review** — mutation-check + adversarial probes; fix, up to 3 rounds, then
+- **Diagrams** — inline SVG koan + one per example.
+
+If your runtime provides a batch/orchestration runner (e.g. `workflow.js` in this directory is a
+Claude Code `Workflow` script that runs both tracks in parallel and resumes crashed agents from
+`journal.jsonl`), use it with `slug/name/number/total/brief/scratch/repo/seeds/examples` args — but
+any agent can run the phases itself in order (research → citation checks → examples → review →
+diagrams), which is exactly what `workflow.js` does under the hood. While research runs, write
+`pages/refactorings/<slug>.md` from the extract-method page (next step) and build the site in a
+scratch copy with stub SVGs to catch Liquid errors early.
 
 ## 3. Assemble the page
 
@@ -82,9 +78,25 @@ Rules that came from review, keep them:
   markdown inside raw block HTML, and IALs on numbered items break the list).
 - Source lines ≤ 72 characters or the pane scrolls horizontally on a 1280px screen.
 - No `{{` `}}` `{%` `%}` in any included source (Liquid runs before highlighting).
-- Copy `run.sh` from extract-method unchanged into the new sources directory.
+- Copy `run.sh` from extract-method unchanged into the new sources directory (then adjust it only
+  if the entry has a real reason to, e.g. a `shared/` module — see next bullet).
 - Add the new sources directory to `exclude:` in `_config.yml` (sources are included via
   `include_relative`, not published as static files).
+- **Setup shared across examples lives in `shared/` and is never shown on the page.** If the entry
+  needs shared machinery (an optic library, a hedgehog spec runner, a common data type), put it in
+  `pages/refactorings/<slug>/shared/` and have `run.sh` compile it (`scala-cli run "$d" … "$SHARED"`
+  for Scala, `-i"$SHARED"` for Haskell) — but do NOT `include_relative` it. Each Example
+  `Before/After/Spec` on the page is then the *move itself*, with a one-line note in the page that
+  the shared setup is hidden. The reviewer considers inline setup (e.g. redefining a Lens type in
+  every After) accidental complexity that buries the motivation.
+- **Never hand-write intermediate helpers in an example.** If the example needs a walk over a
+  recursive type, a fold, or a traversal builder, do not define it in the example's `After` —
+  add it to `shared/` (e.g. a `Plated` class with `descend`/`everywhere`, as in eo's "visit
+  across whole trees" recipe) or use a library, and have the example declare only the instance
+  (`which fields recurse`). A hand-rolled `everywhere` in an example buries the motivation: the
+  optic is the reusable thing, not the helper. This is the same rule as the `shared/` bullet
+  above — shared machinery stays off the page — extended to any intermediate helper the
+  example needs.
 - Do not claim totality or purity the code does not have (e.g. `Int` `div` overflow).
 
 ## 4. Verify before the PR
@@ -100,7 +112,8 @@ Rules that came from review, keep them:
 ## 5. Ship
 
 Commit sources + page + data + config on the branch; push (SSH remote works; if `gh auth status`
-fails ask the user to run `! gh auth login -h github.com -p ssh -w`); open the PR with the
+fails ask the user to run `! gh auth login -h github.com -p ssh -w` — or use whichever GitHub
+tool your environment provides); open the PR with the
 reviewer's mutation table and probe summary in the body. The PR preview URL is
 `https://www.constructive.dev/pr-preview/pr-<N>/refactorings/<slug>/`.
 

.claude/skills/refactoring-entry/brief.md

@@ -38,7 +38,9 @@ when it does not apply.
 
 ## Code layout (examples agent writes these; reviewer runs them)
 pages/refactorings/<slug>/
-  run.sh                      # copied unchanged from extract-method; runs every NN-*/ dir in both languages
+  run.sh                      # copied from extract-method; runs every NN-*/ dir in both languages
+  shared/                     # shared setup (optics, spec runner, common types) — compiled in,
+                              # NEVER included on the page; each example is then the move itself
   NN-<ex>/Before.scala        # object Before { ... }   the pre-refactoring program
   NN-<ex>/After.scala         # object After  { ... }   the refactored version (same public entry point signature)
   NN-<ex>/Spec.scala          # hedgehog property: forAll generated inputs, Before.f(x) ==== After.f(x). `@main def spec()`.
@@ -49,7 +51,12 @@ pages/refactorings/<slug>/
   * every file self-contained, readable, SHORT (Before/After 8–25 lines each); lines ≤ 72 characters;
   * NO `{{`, `}}`, `{%`, `%}` sequences anywhere (Liquid would eat them);
   * one-line comment at the top of Before/After saying what the program does; no essay comments;
-  * the Scala and Haskell Befores must be parallel (same shape: if Scala has an inline lambda, so does Haskell).
+  * the Scala and Haskell Befores must be parallel (same shape: if Scala has an inline lambda, so does Haskell);
+  * **never hand-write intermediate helpers in an example** — if the example needs a walk over a
+    recursive type, a fold, or a traversal builder, put it in `shared/` (e.g. a `Plated`
+    class with `descend`/`everywhere`, as in eo's "visit across whole trees" recipe) or use a
+    library, and declare only the type's instance in the example (`which fields recurse`). A
+    hand-rolled helper in an example buries the move under the tool.
 - Scala: Scala 3.3.x, directives at the top of Spec.scala only:
     //> using scala 3.3.4
     //> using dep qa.hedgehog::hedgehog-core:0.14.0

_config.yml

@@ -42,3 +42,4 @@ plugins: [jekyll-paginate, jekyll-seo-tag, jekyll-feed, jekyll-remote-theme]
 # include_relative; do not also publish them as static files.
 exclude:
   - pages/refactorings/extract-method/
+  - pages/refactorings/replace-mutable-fields-with-lenses/

_data/refactorings.yml

@@ -5,7 +5,7 @@
 - group: "Refactorings"
   items:
     - { name: "Extract / Inline method", slug: extract-method }
-    - { name: "Replace mutable fields with lenses" }
+    - { name: "Replace mutable fields with lenses", slug: replace-mutable-fields-with-lenses }
     - { name: "Replace global state with parameter" }
     - { name: "Introduce ReaderT (Kleisli)" }
     - { name: "Replace loop with fold" }