chore(todo): file structural-refactor backlog from 2026-06-28 review

1 files changed, 370 insertions, 0 deletions

diff --git a/todo.org b/todo.org
index 82a889f5e..9246ab385 100644
--- a/todo.org
+++ b/todo.org
@@ -4106,6 +4106,376 @@ Claude Code truncates a colored span without a reset, so the color bleeds down t
 ** TODO [#D] occur/xref font-lock coloring watch :bug:
 =occur= and =xref= enable font-lock themselves, not via =global-font-lock-mode=, so the exclusion fix does not apply and they show source-line fontification on purpose. No action unless a result ever renders with colors that do not match its source buffer, in which case investigate the real mechanism.
 
+** PROJECT [#B] Structural organization followups from 2026-06-28 review :refactor:
+These are the structural findings from the 2026-06-28 Elisp organization
+review that were not already represented in =todo.org=. The already-tracked
+architecture items were intentionally excluded: eager =init.el= load graph,
+module deferral/autoloads, utility consolidation, keymap registration,
+which-key label drift, music/VAMP extraction, and known =dwim-shell= /
+=external-open= duplicates.
+
+*** TODO [#B] Split calendar-sync.el into parser, recurrence, fetch/state, and Org rendering owners :refactor:
+Location: =modules/calendar-sync.el=.
+
+Issue: =calendar-sync.el= is the largest active module in the config
+\(~1,782 lines, ~105 definitions\). It combines several distinct concerns:
+- module config and persistent sync state near =calendar-sync-calendars= and
+  =calendar-sync--state-file=;
+- timezone/date utilities such as =calendar-sync--add-months=,
+  =calendar-sync--add-days=, =calendar-sync--format-timezone-offset=;
+- ICS text parsing and property extraction such as
+  =calendar-sync--normalize-line-endings=, =calendar-sync--unescape-ics-text=,
+  =calendar-sync--get-property=, =calendar-sync--parse-event=;
+- recurrence and exception handling such as
+  =calendar-sync--parse-rrule=, =calendar-sync--expand-recurring-event=,
+  =calendar-sync--collect-recurrence-exceptions=;
+- external fetch / conversion orchestration for =:fetcher 'ics= and
+  =:fetcher 'api= calendars;
+- Org rendering and agenda-facing commands such as =calendar-sync--event-to-org=,
+  =calendar-sync-now=, =calendar-sync-start=, and keymap setup.
+
+The file is internally sectioned, but changing one concern still requires
+loading and navigating the whole parser/fetch/render stack. Tests also have to
+target one very broad module, so failures do not naturally point at one owner.
+
+Fix shape:
+- Keep =calendar-sync.el= as the public entry point, user options, commands,
+  timer lifecycle, and keymap.
+- Extract a dependency-light parser module, e.g. =calendar-sync-ics.el=, for
+  line normalization, property lookup, text cleanup, timestamp parsing, attendee
+  parsing, and event parsing.
+- Extract recurrence logic to =calendar-sync-recurrence.el=:
+  RRULE parsing, date math, expansion, EXDATE filtering, RECURRENCE-ID
+  exception collection/application.
+- Extract source/fetch state to =calendar-sync-source.el= or
+  =calendar-sync-fetch.el=: secret URL resolution, API helper invocation,
+  curl/process execution, per-calendar state persistence.
+- Extract Org output to =calendar-sync-org.el=: org-safe heading/body/property
+  rendering and final file write.
+- Leave a compatibility layer in =calendar-sync.el= requiring the pieces and
+  exporting the existing public commands; avoid renaming public functions unless
+  tests prove all callers are migrated.
+
+Acceptance checks:
+- Existing calendar-sync unit and integration tests still pass.
+- =emacs --batch -Q -L modules --eval "(require 'calendar-sync)"= either loads
+  cleanly or fails only for a documented external dependency.
+- Parser/recurrence tests can require their extracted modules without starting
+  timers, reading private config, or touching the network.
+- The public command names and =C-; g= behavior remain unchanged.
+
+*** TODO [#B] Split video-audio-recording.el into device, process, UI, and command layers :refactor:
+Location: =modules/video-audio-recording.el=.
+
+Issue: =video-audio-recording.el= is a broad workflow module
+\(~1,110 lines, ~53 definitions\). It currently owns all of these in one file:
+- user options and process state;
+- modeline indicator;
+- external dependency checks for =ffmpeg= and =wf-recorder=;
+- PulseAudio/PipeWire discovery and parsing, including
+  =cj/recording--parse-pactl-sources-verbose=,
+  =cj/recording--parse-pactl-sinks-verbose=,
+  =cj/recording--get-sink-apps=;
+- device labels, sorting, and =completing-read= UI;
+- short device test recordings and playback;
+- validation/auto-fix of stale system-audio monitors;
+- ffmpeg / wf-recorder command construction;
+- process start/stop/sentinel lifecycle;
+- public commands and keymap registration.
+
+The section headers make the file navigable, but the ownership boundary is too
+wide. A device-label bug, an ffmpeg command regression, and a modeline/process
+bug all land in the same module, even though the risk and test shape differ.
+
+Fix shape:
+- Keep =video-audio-recording.el= as the public command/keymap entry point and
+  top-level defcustom home.
+- Extract device discovery/parsing to =video-audio-recording-devices.el=:
+  pactl parsers, source/sink availability, device status labels, sink-app
+  lookup, default monitor resolution.
+- Extract command construction and process lifecycle to
+  =video-audio-recording-process.el=: build audio/video commands, start
+  processes, sentinel, graceful shutdown, wait-for-exit.
+- Extract setup/test UI to =video-audio-recording-ui.el=: labeled
+  =completing-read= tables, quick setup, manual selection, active-audio display,
+  short test recordings.
+- Leave modeline integration either in the public module or in a tiny
+  =video-audio-recording-modeline.el= if it grows further.
+
+Acceptance checks:
+- Existing =test-video-audio-recording-*.el= files still pass after moving
+  requires/imports.
+- Device parser tests can require only the device module and do not load keymaps
+  or process start code.
+- Command-construction tests can require only the process module and do not call
+  =pactl=, =ffmpeg=, or =completing-read=.
+- Public commands under =C-; r= keep their current names and behavior.
+
+*** TODO [#B] Split ai-term.el into project/session, display, backend, and command layers :refactor:
+Location: =modules/ai-term.el=.
+
+Issue: =ai-term.el= is now the AI-agent terminal package rather than a small
+launcher. It is ~1,205 lines and mixes four separate systems:
+- project discovery and candidate ordering:
+  =cj/--ai-term-candidates=, =cj/--ai-term-project-root-p=,
+  =cj/--ai-term-sort-candidates=, =cj/--ai-term-format-candidate=;
+- tmux session naming and live-session discovery:
+  =cj/--ai-term-tmux-session-name=, =cj/--ai-term-live-tmux-sessions=,
+  =cj/--ai-term-session-active-p=, =cj/--ai-term-launch-command=;
+- display/window geometry policy:
+  =cj/--ai-term-reuse-existing-agent=, =cj/--ai-term-reuse-edge-window=,
+  =cj/--ai-term-display-saved=, display-buffer rule setup, toggle-off
+  restore/swap behavior;
+- EAT backend/process interaction:
+  =eat= creation, =eat-buffer-name= binding, pty writes, EAT keymap
+  integration;
+- public commands and shutdown/wrap-up hooks:
+  =cj/ai-term=, =cj/ai-term-pick-project=, =cj/ai-term-next=,
+  =cj/ai-term-close=, =cj/ai-term-quit=, =cj/ai-term-live-count=,
+  =cj/ai-term-shutdown-countdown=.
+
+The existing file has strong tests and comments, so this is not an emergency.
+The structural risk is that future changes to tmux lifecycle, display geometry,
+or project discovery all happen in the same large file and can accidentally
+couple to each other.
+
+Fix shape:
+- Keep =ai-term.el= as the public API, user options, keymap, and compatibility
+  require point.
+- Extract project/session helpers to =ai-term-sessions.el=:
+  candidate discovery, tmux names, live tmux parsing, launch-command building,
+  active-agent-dir rotation inputs.
+- Extract display/window policy to =ai-term-display.el=:
+  display-buffer action functions, toggle capture/restore state, working-buffer
+  swap, display rule list.
+- Extract EAT/tmux buffer backend to =ai-term-backend-eat.el=:
+  create/reattach buffer, pty send helper, EAT keymap integration. Keep the name
+  backend-specific so a future backend swap is localized.
+- Keep shutdown/wrap-up commands either in =ai-term.el= or a small
+  =ai-term-shutdown.el= if they continue to grow.
+
+Acceptance checks:
+- Existing =test-ai-term-*.el= coverage remains green.
+- Session/project tests can require the session module without loading EAT.
+- Display tests can stub session/backend functions and exercise only window
+  behavior.
+- The public =C-; a= commands and =M-SPC= binding continue to resolve exactly
+  as before.
+
+*** TODO [#B] Define and apply a module layout convention for pure helpers, state/process code, commands, and keymaps :refactor:
+Locations: recurring pattern across modules; representative examples:
+=modules/custom-ordering.el=, =modules/custom-text-enclose.el=,
+=modules/calendar-sync.el=, =modules/video-audio-recording.el=,
+=modules/ai-term.el=, =modules/org-webclipper.el=.
+
+Issue: The config has a good recurring pattern in several small command modules:
+pure helpers first, thin interactive wrappers next, keymap/footer last.
+=custom-ordering.el= is a good example: internal transform helpers like
+=cj/--arrayify= and =cj/--unarrayify= are separate from commands like
+=cj/arrayify=, and the keymap sits at the bottom.
+
+The pattern is not explicit, and larger modules drift:
+- some public commands are interleaved with low-level helpers;
+- process/network/shell functions sit beside parser functions;
+- keymap/footer blocks vary in naming and location;
+- lazy =require= forms sometimes appear mid-file without a clear convention;
+- state variables, defcustoms, and implementation constants are not always
+  grouped the same way.
+
+Fix shape:
+- Write a short local convention, probably in =docs/design/module-structure.org=
+  or a section of an existing architecture spec. Recommended order:
+  1. package header, Commentary, Code;
+  2. hard =require= forms and declarations;
+  3. defgroup/defcustom/defface;
+  4. internal state variables;
+  5. pure helpers with no buffer/process/UI side effects;
+  6. state/process/IO helpers;
+  7. interactive commands;
+  8. keymap registration and which-key labels;
+  9. provide footer.
+- Document exceptions: package =use-package= blocks may sit near the commands
+  they configure if that is clearer; lazy =require= belongs at the call site
+  only when it avoids a real startup dependency and has a comment naming why.
+- Apply the convention opportunistically to 2-3 representative modules first:
+  one small pure command module, one medium integration module, and one large
+  process module.
+
+Acceptance checks:
+- The convention document names real examples from this repository.
+- At least one module refactor lands with no behavior changes and only focused
+  tests/byte-compile verification.
+- Future module reviews can cite the convention instead of re-arguing layout.
+
+*** TODO [#B] Normalize Elisp package structure conventions across owned and generated files :refactor:test:
+Locations found during the 2026-06-28 pass:
+- =modules/auth-config.el:1= starts with =;; auth-config.el ---= instead of a
+  package header beginning with =;;;=.
+- =modules/dwim-shell-config.el:1= starts with =;; dwim-shell-config.el ---=.
+- =modules/local-repository.el:1= has a BOM before the =;;;= header.
+- =custom/c-boxes.el:1= has no normal package header, Commentary marker, Code
+  marker, or provide footer.
+- =calendar-sync.local.el:1= has a package header/footer shape but no
+  =;;; Commentary:= or =;;; Code:= markers.
+- =browser-choice.el:1= is generated with =;;; Browser choice - Auto-generated=
+  and no lexical-binding/canonical package header.
+
+Issue: The load-graph header contract in =todo.org= covers the custom module
+metadata block, but it does not cover normal Elisp package conventions. These
+files still break tooling expectations for package headers, generated-file
+headers, Commentary/Code markers, and sometimes byte-compile/checkdoc scans.
+
+Fix shape:
+- Add a lightweight convention/test that checks repository-owned active Elisp
+  files for:
+  - first line matching =;;; NAME.el --- SUMMARY -*- ... -*-=, unless explicitly
+    exempted as vendored/generated/private;
+  - =;;; Commentary:= before =;;; Code:=;
+  - =provide= footer where the file is require-able;
+  - no BOM before the package header.
+- Fix owned files directly:
+  - =auth-config.el= and =dwim-shell-config.el=: change first line to =;;;=.
+  - =local-repository.el=: remove the BOM and keep a normal header.
+  - =calendar-sync.local.el=: add Commentary/Code markers or explicitly exempt
+    private local config from package checks.
+- Fix generators for generated files:
+  - =modules/browser-config.el= should generate a proper header for
+    =browser-choice.el=, e.g.
+    =;;; browser-choice.el --- Generated browser selection -*- lexical-binding: t; -*-=.
+  - =scripts/theme-studio/build-theme.el= should continue to generate a proper
+    header for theme files and should be included/exempted by policy.
+- Decide whether =custom/c-boxes.el= is owned enough to normalize or vendored
+  enough to exempt.
+
+Acceptance checks:
+- New or updated structural test fails on the malformed files before the fix.
+- The test has a short, explicit exemption list for true vendor/generated/private
+  files.
+- =make test= or the relevant architecture/header test passes after fixes.
+
+*** TODO [#C] Split or rehome the unrelated commands in custom-misc.el :refactor:
+Location: =modules/custom-misc.el=.
+
+Issue: =custom-misc.el= is named as a miscellaneous bucket and currently holds
+unrelated behavior:
+- delimiter navigation: =cj/jump-to-matching-paren=;
+- buffer/region formatting: =cj/--format-region= and =cj/format-region-or-buffer=;
+- previous-buffer toggle: =cj/switch-to-previous-buffer=;
+- word/character counts: =cj/--count-words=, =cj/count-words-buffer-or-region=,
+  =cj/--count-characters=, =cj/count-characters-buffer-or-region=;
+- fraction glyph conversion: =cj/--replace-fraction-glyphs=,
+  =cj/replace-fraction-glyphs=;
+- global =align-regexp= advice: =cj/align-regexp-with-spaces=.
+
+Each function is small, but the module has no coherent ownership rule beyond
+"small things that did not fit elsewhere." That makes future additions likely
+to land here by default, and it hides the fact that some pieces have better
+homes already.
+
+Fix shape:
+- Rehome formatting to a shared formatting helper/module if it overlaps with
+  =cj/format-region-with-program= or language format commands; otherwise keep
+  a small =custom-format.el=.
+- Move counts to a =custom-counts.el= or into a text/prose utility module.
+- Move delimiter jump to a navigation/editing module, possibly
+  =custom-line-paragraph.el= or a new =custom-navigation.el= depending on
+  desired keymap ownership.
+- Move fraction glyph conversion to a text transform module, likely
+  =custom-case.el= or a new =custom-text-transform.el=.
+- Keep a compatibility shim in =custom-misc.el= temporarily if external callers
+  exist, but stop adding new commands there.
+
+Acceptance checks:
+- Search for all =cj/= symbols moved from =custom-misc.el= and update callers,
+  tests, and keymap registrations.
+- Existing =test-custom-misc-*.el= tests either move with their functions or
+  become compatibility tests.
+- =custom-misc.el= is deleted or reduced to a short compatibility module with a
+  sunset note.
+
+*** TODO [#C] Define a policy for vendor, archive, generated, and private Elisp files :refactor:
+Locations affected:
+- vendored/local third-party files under =custom/=, e.g. =custom/c-boxes.el=,
+  =custom/elpa-mirror.el=, =custom/edit-indirect.el=, =custom/titlecase.el=,
+  =custom/titlecase-data.el=, =custom/eplot.el=;
+- archived GPTel code under =archive/gptel/=;
+- generated files such as =themes/WIP-theme.el= and =browser-choice.el=;
+- private local config such as =calendar-sync.local.el=.
+
+Issue: Structural and documentation audits currently treat all Elisp files as
+one population unless the reviewer manually remembers which files are owned,
+vendored, archived, generated, or private. That produces noisy findings and
+unclear expectations:
+- Should vendored files be normalized to house package/comment conventions, or
+  kept close to upstream?
+- Should archived code be fixed, exempted, or ignored unless restored?
+- Should generated files be fixed in the generated output or in the generator?
+- Should private local config follow package conventions or a smaller template?
+
+Fix shape:
+- Add a policy document or a small section in the architecture docs that defines
+  file classes:
+  - Owned active config: must follow module/package/header/test conventions.
+  - Vendored code: prefer upstream shape; local changes should be minimal and
+    documented.
+  - Archived code: excluded from active architecture checks unless the task is
+    explicitly about archive cleanup.
+  - Generated code: source of truth is the generator/input; fix generator output,
+    not hand-edited generated files.
+  - Private local config: must avoid secrets and should have a minimal readable
+    header, but may be exempt from require/provide expectations.
+- Add or update architecture/header tests to consume that classification rather
+  than relying on ad hoc file globs.
+- Put the classification in a machine-readable place if practical, e.g. a plist
+  in the test file, an Org table in =docs/design/module-inventory.org=, or a
+  small =docs/design/file-classes.org= that tests can parse later.
+
+Acceptance checks:
+- A reviewer can tell whether =custom/titlecase.el=, =archive/gptel/modules/*=,
+  =themes/WIP-theme.el=, =browser-choice.el=, and =calendar-sync.local.el= are
+  in or out of active structural checks without guessing.
+- At least one architecture/header test uses the policy or exemption list.
+- Generated-file fixes happen in the generator where applicable.
+
+*** TODO [#C] Audit public/private naming boundaries across config-owned Elisp :refactor:
+Locations and examples:
+- =modules/local-repository.el= defines unprefixed =car-member= and
+  =localrepo-initialize=.
+- =modules/external-open.el= defines =default-open-extensions= without a
+  =cj/= or package prefix.
+- Package-private helpers vary between =cj/--name= and package-specific
+  =calendar-sync--name= / =mouse-trap--name= styles.
+- Some modules expose public command names where a private helper would be safer,
+  while others use =cj/--= for helpers that are tested across files.
+
+Issue: Naming mostly works in practice, but the boundary is not explicit. In a
+large personal config loaded into one Emacs namespace, unprefixed symbols and
+unclear private/public boundaries increase collision risk and make ownership
+harder to infer. The =local-repository.el= case is already partly logged as a
+module-specific cleanup in =todo.org=, but the broader convention is not.
+
+Fix shape:
+- Write a short naming convention:
+  - user-facing commands and shared helpers use =cj/...=;
+  - private helpers inside =cj=-owned modules use =cj/--...= unless the module
+    is intentionally package-like;
+  - package-like standalone modules use =package--...= for internals and
+    =package-...= for public API;
+  - no new unprefixed defvars/defuns in owned config modules;
+  - tested private helpers may remain private, but test files should make that
+    intentional rather than treating them as public API.
+- Run a mechanical scan for owned files:
+  =rg -n "^\\((defun|defvar|defcustom|defconst|defgroup) [^ )/]+\\b" modules custom=,
+  excluding true package/vendor files by the file-class policy above.
+- Rename only clear low-risk cases first, with compatibility aliases where
+  external callers may exist.
+
+Acceptance checks:
+- The scan produces either no unexpected unprefixed symbols or a documented
+  allowlist.
+- Renames are covered by focused tests or alias tests.
+- The convention is referenced by future module review tasks.
+
 * Emacs Someday/Maybe
 
 ** TODO [#D] GPTel orphan tasks and useful ideas :feature: