Shared picks namespace (walker moved, remove-at/put-at); lint clean

CHANGELOG.md

@@ -1172,6 +1172,17 @@ one 10px gutter, and the header's tab menus open on screen again.
 
 - **The security-policy docs match the server** — there is no report-only mode; development sends no policy, everything else enforces it (`4e972cf7`, `c9fd5a5b`).
 
+### refactor/picks-namespace
+
+**Added**
+
+- **Remove and put back one stored pick by its address** — `picks/remove-at` and `picks/put-at`, the base for repairing a character and for setting picks aside (`439593dd`, `b33c765a`).
+
+**Changed**
+
+- **Lint is clean** — the 50 warnings and 3 infos on `integration` are fixed, not suppressed: shadowing locals renamed for what they hold, unused requires removed, nested lets merged; no behaviour change (`ad3c612b`).
+- **The pick walker moved into shared code** — `walk-entries`, `walk-picks`, `picks-of` and `relink-picks` now live in `orcpub.dnd.e5.picks` (as `walk`, `walk-typed`, `keys-of`, `relink`), unchanged, so the server can use them and their tests run in CI (`439593dd`).
+
 ## [breaking/2026-stack-modernization]
 
 ### Infrastructure

src/clj/orcpub/config.clj

@@ -271,16 +271,17 @@
    change was picked up, ignored as a typo, or never set. Secret values are dropped here, not
    at print time, so nothing downstream can leak one by accident."
   []
-  (for [{:keys [var get secret? redact? critical? note group fix]} settings]
-    (let [raw      (env-raw var)
-          integer? (some? (some #(= var (:var %)) tunables))
-          parsed   (when (and raw integer?)
-                     (try (Integer/parseInt (str/trim raw)) (catch NumberFormatException _ nil)))
-          ignored? (boolean (and raw integer? (not (and parsed (pos? parsed)))))
-          value    (cond secret? nil
-                         get     (get)
-                         :else   raw)]
-      {:var var :group group :note note :critical? critical? :fix fix
+  (for [{env-var :var lookup :get :keys [secret? redact? critical? note group fix]} settings]
+    (let [raw              (env-raw env-var)
+          integer-setting? (some? (some #(= env-var (:var %)) tunables))
+          parsed           (when (and raw integer-setting?)
+                             (try (Integer/parseInt (str/trim raw))
+                                  (catch NumberFormatException _ nil)))
+          ignored?         (boolean (and raw integer-setting? (not (and parsed (pos? parsed)))))
+          value            (cond secret? nil
+                                 lookup  (lookup)
+                                 :else   raw)]
+      {:var env-var :group group :note note :critical? critical? :fix fix
        :value (if (and value redact?) (redact-secrets value) value)
        :secret? secret?
        :raw (when-not secret? raw)
@@ -298,10 +299,10 @@
   ([rows actual]
    (let [;; A secret is reported as present or absent and NEVER by value. `-` where we have
          ;; no number at all, so the VALUE column never argues with SOURCE beside it.
-         val    (fn [{:keys [var value secret? set?]}]
+         val    (fn [{env-var :var :keys [value secret? set?]}]
                   (cond secret?          (if set? "set" "NOT SET")
                         value            (str value)
-                        (get actual var) (str (get actual var))
+                        (get actual env-var) (str (get actual env-var))
                         :else            "-"))
          ;; A secret's VALUE column already says set / NOT SET; repeating DEFAULT beside it
          ;; says nothing, and "DEFAULT" next to "NOT SET" reads as though there is a
@@ -325,19 +326,19 @@
                 (cons (str "  [" (.toUpperCase ^String (or (:group (first gr)) "other") Locale/ROOT) "]")
                       (map #(trim (format fmt (:var %) (val %) (source %) (or (:note %) ""))) gr)))
               (partition-by :group rows))
-      [(str "  [BRANDING]")
+      ["  [BRANDING]"
        (format "  %s   %s of %s set" (apply str (repeat w " ")) brand (count branding-vars))]
       (when-let [bad (seq (filter :ignored? rows))]
         (cons rule
-              (for [{:keys [var raw]} bad]
+              (for [{env-var :var :keys [raw]} bad]
                 (format "  (!)  %s=%s was ignored: not a positive integer. The default above is in use."
-                        var raw))))
+                        env-var raw))))
       ;; A missing SIGNATURE means every login fails. That is not a row to be spotted.
       ;; Say what broke AND how to fix it. A boot line that names a symptom and leaves the
       ;; remedy to be guessed just moves the work.
       (when-let [miss (seq (filter #(and (:critical? %) (not (:set? %))) rows))]
-        (mapcat (fn [{:keys [var note fix]}]
-                  (let [body (str/split-lines (or fix (str var " is NOT SET -- " (or note "required"))))]
+        (mapcat (fn [{env-var :var :keys [note fix]}]
+                  (let [body (str/split-lines (or fix (str env-var " is NOT SET -- " (or note "required"))))]
                     (cons rule
                           ;; Continuations line up under the first line's text, not under the
                           ;; marker, so the block reads as one paragraph.

src/clj/orcpub/pdf.clj

@@ -21,7 +21,7 @@
                                                               PDAppearanceStream)
            (org.apache.pdfbox.pdmodel.common PDRectangle)
            (org.apache.pdfbox.cos COSName COSDictionary)
-           (org.apache.pdfbox.pdmodel PDPage PDDocument PDPageContentStream PDResources)
+           (org.apache.pdfbox.pdmodel PDPage PDPageContentStream PDResources)
            ;; APPEND appends operators to a page's existing content stream, so
            ;; template artwork survives. The alternative overwrites it.
            (org.apache.pdfbox.pdmodel PDPageContentStream$AppendMode)

src/clj/orcpub/styles/core.clj

@@ -804,7 +804,7 @@
     ;; The outer shadow is a low glow, not a selected-field highlight.
     {:position :relative
      :border-color :transparent
-     :box-shadow (str "0 0 14px rgba(240, 161, 0, 0.13)")}
+     :box-shadow "0 0 14px rgba(240, 161, 0, 0.13)"}
     [:&:before
      {:content "''"
       :position :absolute

src/cljc/orcpub/dnd/e5/library.cljc

@@ -45,12 +45,12 @@
           {} (content-groups plugins)))
 
 (defn broken-links
-  "Links in `new` that name an item `old` held and `new` does not, as
+  "Links in `after` that name an item `before` held and `after` does not, as
    [{:source :type :key :name :target :target-type}]. Links to a [type key] in `retargeting` are
    being moved elsewhere and are not counted."
-  [old new retargeting]
-  (let [was (held old) now (held new)]
-    (for [[src ct items] (content-groups new)
+  [before after retargeting]
+  (let [was (held before) now (held after)]
+    (for [[src ct items] (content-groups after)
           [k item] items
           :when (map? item)
           link links/links
@@ -63,29 +63,29 @@
       {:source src :type ct :key k :name (:name item) :target target :target-type to})))
 
 (defn- invalid-keys
-  "[source type key] of every item `new` stores under a key the loader would set aside, that
-   `old` did not already hold there."
-  [old new]
-  (for [[src ct items] (content-groups new)
+  "[source type key] of every item `after` stores under a key the loader would set aside, that
+   `before` did not already hold there."
+  [before after]
+  (for [[src ct items] (content-groups after)
         k (keys items)
         :when (and (not (common/keyword-starts-with-letter? k))
-                   (not (contains? (get-in old [src ct]) k)))]
+                   (not (contains? (get-in before [src ct]) k)))]
     [src ct k]))
 
 (def ^:private built-in-names
   "{content-type #{name}} of built-in content a `:by :name` link can name."
   {:orcpub.dnd.e5/languages (into #{} (map :name) langs/languages)})
 
 (defn- follow-renamed-names
-  "`new` with each `:by :name` link naming an item `old` knew by another name renamed to match,
-   when nothing in `new` or built-in content still answers to the old name."
-  [old new]
+  "`after` with each `:by :name` link naming an item `before` knew by another name renamed to match,
+   when nothing in `after` or built-in content still answers to the old name."
+  [before after]
   (let [names (reduce (fn [acc [_ ct items]]
                         (update acc ct (fnil into #{}) (keep :name) (filter map? (vals items))))
-                      built-in-names (content-groups new))
-        renames (for [[src ct items] (content-groups new)
+                      built-in-names (content-groups after))
+        renames (for [[src ct items] (content-groups after)
                       [k item] items
-                      :let [was (get-in old [src ct k :name])]
+                      :let [was (get-in before [src ct k :name])]
                       :when (and (string? was) (string? (:name item)) (not= was (:name item))
                                  (not (contains? (get names ct) was)))]
                   [ct was (:name item)])
@@ -100,27 +100,27 @@
                                                                   item name-links)))
                                              {} items)))
                       p (content-groups p)))
-            new renames)))
+            after renames)))
 
 (defn commit
-  "What storing `new` in place of `old` does: {:plugins (normalize new) :broken [...]}, or
+  "What storing `after` in place of `before` does: {:plugins (normalize after) :broken [...]}, or
    {:refused {:broken [...] :invalid [...]}} when it would add a key the loader sets aside or
    leave a link pointing at nothing. `deleting?` lets a deliberate removal through and reports
    the links it broke; `retargeting` is a collection of [type key] whose links are being moved;
    `restoring?` lets keys the loader sets aside through (it sets them aside again)."
-  [old new {:keys [deleting? retargeting restoring?]}]
-  (let [stored (follow-renamed-names old (normalize new))
-        broken (vec (broken-links old stored (set retargeting)))
-        invalid (if restoring? [] (vec (invalid-keys old stored)))]
+  [before after {:keys [deleting? retargeting restoring?]}]
+  (let [stored (follow-renamed-names before (normalize after))
+        broken (vec (broken-links before stored (set retargeting)))
+        invalid (if restoring? [] (vec (invalid-keys before stored)))]
     (if (or (seq invalid) (and (seq broken) (not deleting?)))
       {:refused {:broken broken :invalid invalid}}
       {:plugins stored :broken broken})))
 
 (defn overwritten
-  "Items `old` and `new` both hold at the same source, type and key, whose content differs, as
-   [{:source :type :key :name}]. The name is the one `new` gives it."
-  [old new]
-  (let [o (normalize old) n (normalize new)]
+  "Items `before` and `after` both hold at the same source, type and key, whose content differs, as
+   [{:source :type :key :name}]. The name is the one `after` gives it."
+  [before after]
+  (let [o (normalize before) n (normalize after)]
     (for [[src ct items] (content-groups n)
           [k item] items
           :let [was (get-in o [src ct k])]
@@ -354,13 +354,13 @@
   "Picks in `character`'s options, under a choice tagged with one of `tags` (per `choice-tags`),
    that `offered` lacks, as [{:key :selection :tag}]. Skips entries carrying their own
    `::entity/value`."
-  [character offered choice-tags tags]
+  [character offered tags-by-selection tags]
   (letfn [(walk [opts]
             (for [[sel v] opts
                   e (if (sequential? v) v [v])
                   :when (map? e)
                   x (cons (let [k (::entity/key e)
-                                tag (some tags (get choice-tags sel))]
+                                tag (some tags (get tags-by-selection sel))]
                             (when (and tag (keyword? k)
                                        (not (contains? e ::entity/value))
                                        (not (offers? offered k)))

src/cljc/orcpub/dnd/e5/library_links.cljc

@@ -100,19 +100,19 @@
             x)))
 
 (defn retarget
-  "`item` with every target of `link` equal to `old` replaced by `new`. Leaves the item unchanged
+  "`item` with every target of `link` equal to `before` replaced by `after`. Leaves the item unchanged
    where it does not hold the link.
    GOTCHA: a `:by :name` link is left alone; renaming a key does not change a name."
-  [link item old new]
+  [link item before after]
   (if (= :name (:by link))
     item
-    (rewrite item (:path link) #(if (= % old) new %))))
+    (rewrite item (:path link) #(if (= % before) after %))))
 
 (defn rename-target
-  "`item` with every name `old` that the `:by :name` `link` holds replaced by `new`."
-  [link item old new]
+  "`item` with every name `before` that the `:by :name` `link` holds replaced by `after`."
+  [link item before after]
   (if (= :name (:by link))
-    (rewrite item (:path link) #(if (= % old) new %))
+    (rewrite item (:path link) #(if (= % before) after %))
     item))
 
 (def former-key-cap
@@ -142,17 +142,17 @@
                               (into [(first ks)] (take-last (dec former-key-cap) ks)))))))
 
 (defn repoint
-  "`plugin` (one source, {content-type {key item}}) with every link naming `old` in `to-type`
-   retargeted to `new`, in every item that can hold it."
-  [plugin to-type old new]
+  "`plugin` (one source, {content-type {key item}}) with every link naming `before` in `to-type`
+   retargeted to `after`, in every item that can hold it."
+  [plugin to-type before after]
   (let [relevant (filter #(= to-type (:to %)) links)]
     (reduce-kv
      (fn [p ct items]
        (if (and (qualified-keyword? ct) (map? items))
          (assoc p ct (reduce-kv
                       (fn [m k item]
                         (assoc m k (reduce #(if (and (map? %1) (holds? %2 ct))
-                                              (retarget %2 %1 old new)
+                                              (retarget %2 %1 before after)
                                               %1)
                                            item relevant)))
                       {} items))

src/cljc/orcpub/dnd/e5/picks.cljc

@@ -0,0 +1,121 @@
+(ns orcpub.dnd.e5.picks
+  "A character's stored picks (the entries under `::entity/options`): walking, reading, rewriting,
+   removing and putting back. Pure data, shared by the browser and the server. A selection stores
+   its picks as a vector when it is `::t/multiselect?` (kept, empty, after its last pick goes),
+   otherwise as the one entry map; an absent selection was never chosen.
+   GOTCHA: never depend on the iteration order of a set or map here; it differs between the JVM
+   and the browser (`testing-infrastructure.md`)."
+  (:require [orcpub.entity :as entity]
+            [orcpub.dnd.e5.library :as library]))
+
+(defn walk
+  "`opts` (::entity/options) with `(f path entry)` applied to every chosen entry, where `path` is
+   the vector of selection keys from the root down to it."
+  [opts f]
+  (letfn [(entry [path e]
+            (if (map? e)
+              (let [e' (f path e)]
+                (cond-> e' (map? (::entity/options e')) (update ::entity/options #(walk path %))))
+              e))
+          (walk [path o]
+            (into {} (map (fn [[sel v]]
+                            (let [p (conj path sel)]
+                              [sel (if (sequential? v) (mapv #(entry p %) v) (entry p v))])))
+                  o))]
+    (if (map? opts) (walk [] opts) opts)))
+
+(defn- walk-typed
+  "`opts` (::entity/options) with `f` applied to every pick that could be of `content-type`
+   (`library/pick-types`); `content-type` nil: every pick."
+  [opts content-type f]
+  (walk opts (fn [path e]
+               (if (or (nil? content-type) (seq (library/pick-types path [content-type])))
+                 (f e)
+                 e))))
+
+(defn keys-of
+  "Set of `character`'s pick keys that could be of `content-type` (see `walk-typed`)."
+  [character content-type]
+  (let [ks (atom #{})]
+    (walk-typed (::entity/options character) content-type
+                #(do (when-let [k (::entity/key %)] (swap! ks conj k)) %))
+    @ks))
+
+(defn relink
+  "`character` with each pick of `from` that could be of `content-type` rewritten to `to`, as
+   {:character :rewrote} (see `content-reconciliation/reconcile-former-keys`). `content-type` nil:
+   every pick."
+  [character content-type from to]
+  (let [rewrote (atom [])
+        opts (walk-typed (::entity/options character) content-type
+                         #(if (= from (::entity/key %))
+                            (do (swap! rewrote conj {:from from :to to}) (assoc % ::entity/key to))
+                            %))]
+    {:character (cond-> character (seq @rewrote) (assoc ::entity/options opts))
+     :rewrote @rewrote}))
+
+;; An address names one pick: [selection key, chosen entry's key] pairs from the root, the format
+;; of the app's option paths (`entity/get-entity-path`), e.g. [:class :ranger :levels :level-7].
+;; A key, never an index: removing one pick must not move another's address.
+
+(defn- entry-index [v k]
+  (first (keep-indexed (fn [i e] (when (= k (::entity/key e)) i)) v)))
+
+(defn- options-path
+  "The `get-in` path to the ::entity/options map holding the last pair of `address`, or nil when
+   an entry on the way is not stored."
+  [character address]
+  (loop [path [::entity/options] [[sel k] & more] (partition 2 address)]
+    (if (empty? more)
+      path
+      (let [v (get-in character (conj path sel))
+            step (if (sequential? v)
+                   (when-let [i (entry-index v k)] [sel i])
+                   (when (= k (::entity/key v)) [sel]))]
+        (when step (recur (into path (conj step ::entity/options)) more))))))
+
+(defn remove-at
+  "`character` without the pick at `address`, as {:character :removed}; `:removed` is
+   {:address :entry :multiselect?}, what `put-at` takes, or nil when nothing is stored there.
+   A multiselect keeps its vector, emptied if this was its last pick, as unticking does; a single
+   pick's selection is dissoc'd. `:multiselect?` records which, since neither shows it after."
+  [character address]
+  {:pre [(even? (count address)) (seq address)]}
+  (let [sel (nth address (- (count address) 2))
+        k (last address)
+        opath (options-path character address)
+        v (when opath (get-in character (conj opath sel)))]
+    (cond
+      (and (sequential? v) (entry-index v k))
+      {:character (assoc-in character (conj opath sel)
+                            (with-meta (vec (remove #(= k (::entity/key %)) v)) (meta v)))
+       :removed {:address address :entry (nth v (entry-index v k)) :multiselect? true}}
+
+      (and (map? v) (= k (::entity/key v)))
+      {:character (update-in character opath dissoc sel)
+       :removed {:address address :entry v :multiselect? false}}
+
+      :else {:character character :removed nil})))
+
+(defn put-at
+  "`character` with `removed` (from `remove-at`) stored again at its address: appended to a
+   `:multiselect?` vector (created if absent), else as the selection's entry. Nil when nothing is
+   written: an entry on the way is gone, the vector already holds its key, or the one-pick
+   selection already holds a pick. Never overwrites or duplicates a pick.
+   GOTCHA: checks no rule; whether it still fits is the caller's question."
+  [character {:keys [address entry multiselect?]}]
+  (let [sel (nth address (- (count address) 2))
+        opath (options-path character address)
+        v (when opath (get-in character (conj opath sel)))]
+    (cond
+      (nil? opath) nil
+      multiselect? (cond (not (sequential? v))
+                         (assoc-in character (conj opath sel) [entry])
+
+                         (entry-index v (::entity/key entry)) nil
+
+                         :else
+                         (assoc-in character (conj opath sel)
+                                   (with-meta (conj (vec v) entry) (meta v))))
+      (some? v) nil
+      :else (assoc-in character (conj opath sel) entry))))