Restructure src/ to package-by-feature with shared kernel + DI

[pajoma]

Summary

Restructure src/ from package-by-layer (commands/, journal/, model/, vscode/, features/, ui/, util/) to package-by-feature with a thin shared kernel. Goal: one feature lives in one place, clear separation of concerns, real dependency injection, and a vscode-free domain layer. No user-facing behavior change.

Aligns with docs/PLAN.md Phase 2+ (DI, named imports, native async/await, vscode.workspace.fs).

📄 Full spec: docs/specs/2026-06-02-234-package-by-feature.md · Plan: docs/plans/2026-06-02-234-package-by-feature.md

Problem

A single feature is smeared across many directories. "Notes" lives in commands/show-note.ts, journal/parser.ts, journal/writer.ts, features/entries/load-note.ts, features/sync/sync-note-links.ts, and vscode/conf.ts — six directories for one capability.

Concrete smells:

1. J namespace barrel = circular god-import. src/index.ts re-exports every submodule as J.Util, J.Journal, etc. Files do import * as J from '..' then J.Util.Ctrl. Those files are themselves re-exported by index.ts, so index.ts → commands/show-note.ts → index.ts is a cycle. Kills tree-shaking, breaks go-to-definition, hides dependencies.
2. Ctrl service-locator god-object (util/controller.ts). Every command/provider receives the full Ctrl and reaches ctrl.reader/ctrl.writer/ctrl.ui/ctrl.config/ctrl.inject. Narrow interfaces (IReader, …) exist in model/interfaces.ts but are unused at call sites. Two-phase init forces ! assertions on every field. Controller also lives in util/ — wrong home.
3. Domain coupled to vscode. Reader/Writer/Inject import vscode and return vscode.TextDocument, so core logic can't be unit-tested without the Extension Host.
4. Configuration god-class — 708 lines (vscode/conf.ts), ~40 public methods mixing raw setting reads, path-pattern resolution, template lookup, scope resolution, windows-path normalization, and local-vs-remote variants.
5. Inconsistent buckets. No rule separates journal/ vs features/ vs ui/. scan-entries is a "feature" but reader is "journal"; codeactions is in ui but sync is in features; two template modules split across layers.
6. Orchestration leaks into commands. show-entry-for-date.ts loadPageForInput runs fire-and-forget weekly sync via nested .then/.catch.
7. Mixed async styles — .then() chains alongside async/await.

Proposed target structure

src/
  extension.ts            # activate/deactivate only
  app/                    # composition root (was vscode/startup + util/controller)
    container.ts          # builds the dependency graph — replaces Ctrl service-locator
    register.ts           # command/provider registration
    startup.ts            # syntax highlighting, cache-invalidation wiring
  shared/                 # kernel — NO feature dependencies
    config/               # split Configuration: SettingsReader | PathResolver | TemplateProvider
    fs/                   # IFileSystem + VscodeFileSystem
    logging/  dates/  strings/
    templates/            # merge template-engine + template-service
    editor/               # the single vscode-TextDocument adapter (open/show/save)
    events/               # typed EventEmitter / mediator for cross-feature signals
    model/                # shared types: Input, FileEntry, ScopedTemplate
  features/
    entries/              # today | tomorrow | yesterday | date | input + entry reader/writer
    notes/                # show-note + load-note + note-path + sync-note-links
    weekly/               # weekly entry + watcher + sync-daily-links
    tasks/                # copy/shift + codeactions + migrate codelens
    navigation/           # prev/next entry
    smart-input/          # match-input + parser + input dialog
    tools/                # print-time | print-duration | print-sum

Each feature folder: commands/ (VS Code handlers) · domain/ (pure logic, no vscode) · ui/ (providers).

Separation rules

• A feature may import from shared/. A feature must never import another feature's internals.
• Cross-feature interaction goes through shared/events/ — a small typed EventEmitter/mediator. Example: entries emits entryOpened → weekly subscribes and runs daily-link sync. No feature-to-feature imports.
• domain/ imports no vscode; it returns data/paths. commands/ plus the shared/editor/ adapter handle TextDocument.
• Inject narrow interfaces (IReader, not Ctrl) via constructor. app/container.ts builds the graph once.

Service lifecycle

app/container.ts is the single owner of service lifecycles. All services are singletons (one instance per activation), matching the current single-Ctrl model. No separate shared/state/ module unless genuinely mutable cross-feature state appears — out of scope here.

Migration phases (each keeps the suite green)

1. Kill the J barrel → named imports. Mechanical, large diff, zero behavior change. Unblocks everything else.
2. Ctrl → DI container in app/. Commands take narrow interfaces. Remove two-phase ! init.
3. Split Configuration into SettingsReader / PathResolver / TemplateProvider.
4. Move files into feature folders (git mv + fix imports). Rewire onNotesInjected + inline weekly-sync onto shared/events/.
5. Extract shared/editor/ adapter → vscode-free domain/. Highest payoff, highest effort — may split to a follow-up issue.

Preconditions

• Merge pending PRs first (notably feat(match-input): replace regex parser with tokenizer, remove moment (#177, #230) #231, feat(notes): link note to specific day via temporal prefix (#232) #233). Phase 1 touches nearly every file; landing it over open PRs causes large rebase conflicts.

Scope / non-goals

• No user-facing behavior change. No new settings, commands, or templates.
• No change to the journal.* settings schema or file/path patterns in package.json.
• i18n strings unchanged (English-only).
• No new mutable-state module unless a concrete need surfaces.

Acceptance criteria

• npm run check green after every phase.
• No import * as J from remaining; src/index.ts barrel removed or reduced to type-only re-exports.
• No command/provider depends on concrete Ctrl; all take narrow interfaces wired in app/container.ts.
• Configuration split into ≤3 focused classes, none over ~300 lines.
• Each feature directory self-contained; no cross-feature internal imports (grep/lint verifiable).
• Cross-feature signals flow through shared/events/.
• docs/PLAN.md updated to reflect the new structure.

[pajoma]

Critical Review of Specification for #234:

The proposed restructuring is a precise remedy for the architectural decay identified in the recent audit.

Strengths:

1. Dependency Disorder: Killing the root barrel (\src/index.ts) and the \J\ namespace is the single most important step to break the global circular dependency.
2. Seam Restoral: Moving from the \Ctrl\ service-locator to a DI container in \�pp/\ restores collapsed seams, enabling true unit testing of commands and features.
3. Domain Cohesion: The 'Package-by-Feature' approach correctly groups the 'Notes' and 'Entries' logic which is currently smeared across 6+ directories.
4. Vscode-free Domain: The goal of making the domain layer VS Code-free (Phase 5) is the correct long-term strategy for stability and testability.

Observations/Recommendations:

• Cross-feature Communication: The spec mentions interaction via 'shared interface or event'. I suggest adding a simple \EventEmitter\ or \Mediator\ to \shared/\ to support this without features importing each other.
• Phase 1 Blast Radius: Phase 1 will touch almost every file. I recommend merging all pending PRs (especially feat(match-input): replace regex parser with tokenizer, remove moment (#177, #230) #231/feat(notes): link note to specific day via temporal prefix (#232) #233) before initiating this to minimize merge hell.
• State Management: Clarify if the DI container will manage the lifecycle of services (singletons) or if a new 'State' module is needed in \shared/.

Approval: Approved. This is a robust plan that will move the codebase from a 'Big Ball of Mud' to a modern, testable architecture.

[pajoma]

Folded the review recommendations into the spec and issue body:

• Cross-feature comms → added shared/events/ (typed EventEmitter/mediator). The current reader.onNotesInjected callback and the inline weekly-sync in show-entry-for-date.ts become published entryOpened events, subscribed by weekly/notes. No feature-to-feature imports.
• DI lifecycle → clarified: app/container.ts is the single owner; all services are singletons (one per activation), matching today's single-Ctrl model. No separate shared/state/ module unless mutable cross-feature state actually appears (out of scope).
• Phase 1 blast radius → recorded as a hard precondition: merge open PRs (feat(match-input): replace regex parser with tokenizer, remove moment (#177, #230) #231, feat(notes): link note to specific day via temporal prefix (#232) #233) before Phase 1, since it touches nearly every file.

Spec/plan committed per the workflow:

• docs/specs/2026-06-02-234-package-by-feature.md
• docs/plans/2026-06-02-234-package-by-feature.md

Maintainer framing & decisions (authorship): the package-by-feature target, the 5-phase ordering (untangle deps → move files → change semantics last), the singleton-no-state-module call, and the merge-PRs-first precondition are maintainer-driven. The assistant's role was the mechanical audit (smell inventory, file/line refs, line counts) and drafting the spec/plan text from those decisions.

[pajoma]

Critical Review of Implementation Plan for #234:

The 5-phase approach is technically excellent and prioritizes stability while systematically addressing architectural rot.

Strengths:

1. Sequential De-risking: Solving dependency cycles (Phase 1) before moving files (Phase 4) is the correct sequence. Attempting both at once is a recipe for broken builds.
2. Facade for Configuration: The decision to maintain the \IConfiguration\ facade in Phase 3 is a great practical choice—it allows splitting the 700-line god-class without forcing a massive rewrite of every consumer simultaneously.
3. Event-driven Cohesion: Using a typed \EventEmitter\ in \shared/events/\ to replace the fire-and-forget sync logic in commands is exactly how cross-feature interaction should work in a modular system.
4. Phased PRs: The rollback strategy (one PR per phase) ensures that we don't have a single 'point of no return' for the entire restructure.

Critical Observations:

• Merge Order: I strongly agree with the precondition to merge feat(match-input): replace regex parser with tokenizer, remove moment (#177, #230) #231 and feat(notes): link note to specific day via temporal prefix (#232) #233 first. Phase 1 will make any existing feature branches almost impossible to rebase.
• Phase 5 Granularity: I concur with splitting Phase 5 (vscode-free domain) into its own follow-up issue. The change in return types (removing \TextDocument) is a semantic shift that deserves its own focus and verification cycle.

Final Approval: Approved. The plan is rigorous, safe, and achieves the architectural goals of the project. Ready to proceed with Phase 1 once open PRs land.

[pajoma]

Phase 1 (kill J barrel) → #235. 47 files, named imports, root src/index.ts emptied. tsc + esbuild + lint green, full suite 238 passing. Behavior-identical.

[pajoma]

Phase 2 (Ctrl service-locator → app/Container DI) → #236. New src/app/ composition root; consumers depend on JournalController interface; two-phase initServices/! removed; IConfiguration expanded; WeeklySyncConfig moved to model. tsc + esbuild + lint green, 238 passing. Depends on #235.

[pajoma]

Phase 3 (split Configuration) → #237. conf.ts 700→105 lines (IConfiguration facade) composing SettingsReader / PathResolver / TemplateProvider (+ patterns types). TemplateService split into the two pure resolvers via IJournalSettings seam. tsc + esbuild + lint green, 238 passing. Depends on #236.