|
| 1 | +# Archived Decisions (consolidated 2026-04-03) |
| 2 | + |
| 3 | +Originals replaced by consolidated entries in DECISIONS.md. |
| 4 | + |
| 5 | +## Group: Output/write location |
| 6 | + |
| 7 | +## [2026-03-22-084509] No runtime pluralization — use singular/plural text key pairs |
| 8 | + |
| 9 | +**Status**: Accepted |
| 10 | + |
| 11 | +**Context**: Hardcoded English plural rules (+ s, y → ies) were scattered across format.Pluralize, padPluralize, and inline code — all i18n dead-ends |
| 12 | + |
| 13 | +**Decision**: No runtime pluralization — use singular/plural text key pairs |
| 14 | + |
| 15 | +**Rationale**: Different languages have vastly different plural rules. Complete sentence templates with embedded counts (time.minute-count '1 minute', time.minutes-count '%d minutes') let each locale define its own plural forms. |
| 16 | + |
| 17 | +**Consequence**: format.Pluralize and format.PluralWord are deleted. All plural output uses paired text keys with the count embedded in the template. |
| 18 | + |
| 19 | +--- |
| 20 | + |
| 21 | +## [2026-03-21-084020] Output functions belong in write/, logic and types in core/ |
| 22 | + |
| 23 | +**Status**: Accepted |
| 24 | + |
| 25 | +**Context**: PrintFeedReport was initially placed in cli/site/core/ but it calls cmd.Println — that's output formatting, not business logic |
| 26 | + |
| 27 | +**Decision**: Output functions belong in write/, logic and types in core/ |
| 28 | + |
| 29 | +**Rationale**: The project taxonomy separates concerns: core/ owns domain logic, types, and helpers; write/ owns CLI output formatting that takes *cobra.Command for Println. Mixing them blurs the boundary and makes testing harder. |
| 30 | + |
| 31 | +**Consequence**: All functions that call cmd.Print/Println/Printf belong in the write/ package tree. core/ never imports cobra for output purposes. |
| 32 | + |
| 33 | +--- |
| 34 | + |
| 35 | + |
| 36 | +## Group: YAML text externalization |
| 37 | + |
| 38 | +## [2026-04-03-133236] YAML text externalization justification is agent legibility, not i18n |
| 39 | + |
| 40 | +**Status**: Accepted |
| 41 | + |
| 42 | +**Context**: Principal analysis initially framed 879-key YAML externalization as a bet on i18n. Blog post review (v0.8.0) revealed the real justification: agent legibility (named DescKey constants as traversable graphs), drift prevention (TestDescKeyYAMLLinkage catches orphans mechanically), and completing the archaeology of finding all 879 scattered strings. |
| 43 | + |
| 44 | +**Decision**: YAML text externalization justification is agent legibility, not i18n |
| 45 | + |
| 46 | +**Rationale**: The v0.8.0 blog makes it explicit: finding strings is the hard part, translation is mechanical. The externalization already pays for itself through agent legibility and mechanical verification. i18n is a free downstream consequence, not the justification. |
| 47 | + |
| 48 | +**Consequence**: Future architecture analysis should frame externalization as already-justified investment. The 3-file ceremony (DescKey + YAML + write/err function) is the cost of agent-legible, drift-proof output — not speculative i18n prep. |
| 49 | + |
| 50 | +--- |
| 51 | + |
| 52 | +## [2026-03-15-101336] TextDescKey exhaustive test verifies all 879 constants resolve to non-empty YAML values |
| 53 | + |
| 54 | +**Status**: Accepted |
| 55 | + |
| 56 | +**Context**: PR #42 merged with ~80 new MCP text keys but no test coverage for key-to-YAML mapping |
| 57 | + |
| 58 | +**Decision**: TextDescKey exhaustive test verifies all 879 constants resolve to non-empty YAML values |
| 59 | + |
| 60 | +**Rationale**: A single table-driven test parsing embed.go source catches typos and missing YAML entries at test time — no manual key list to maintain |
| 61 | + |
| 62 | +**Consequence**: New TextDescKey constants are automatically covered; orphaned keys fail CI |
| 63 | + |
| 64 | +--- |
| 65 | + |
| 66 | +## [2026-03-15-040638] Split text.yaml into 6 domain files loaded via loadYAMLDir |
| 67 | + |
| 68 | +**Status**: Accepted |
| 69 | + |
| 70 | +**Context**: text.yaml grew to 1812 lines covering write, errors, mcp, doctor, hooks, and ui domains |
| 71 | + |
| 72 | +**Decision**: Split text.yaml into 6 domain files loaded via loadYAMLDir |
| 73 | + |
| 74 | +**Rationale**: Matches existing split pattern (commands.yaml, flags.yaml, examples.yaml); loadYAMLDir merges all files in commands/text/ transparently so TextDesc() API stays unchanged |
| 75 | + |
| 76 | +**Consequence**: New domain files must go into commands/text/; loadYAMLDir reads all .yaml in the directory at init time |
| 77 | + |
| 78 | +--- |
| 79 | + |
| 80 | +## [2026-03-12-133007] Split commands.yaml into 4 domain files |
| 81 | + |
| 82 | +**Status**: Accepted |
| 83 | + |
| 84 | +**Context**: Single 2373-line YAML mixed commands, flags, text, and examples with inconsistent quoting |
| 85 | + |
| 86 | +**Decision**: Split commands.yaml into 4 domain files |
| 87 | + |
| 88 | +**Rationale**: Context is for humans — localization files should be human-readable block scalars. Separate files eliminate the underscore prefix namespace hack |
| 89 | + |
| 90 | +**Consequence**: 4 files (commands.yaml, flags.yaml, text.yaml, examples.yaml) with dedicated loaders in embed.go |
| 91 | + |
| 92 | +--- |
| 93 | + |
| 94 | +## [2026-03-06-200257] Externalize all command descriptions to embedded YAML for i18n readiness — commands.yaml holds Short/Long for 105 commands plus flag descriptions, loaded via assets.CommandDesc() and assets.FlagDesc() |
| 95 | + |
| 96 | +**Status**: Accepted |
| 97 | + |
| 98 | +**Context**: Command descriptions were inline strings scattered across 105 cobra.Command definitions |
| 99 | + |
| 100 | +**Decision**: Externalize all command descriptions to embedded YAML for i18n readiness — commands.yaml holds Short/Long for 105 commands plus flag descriptions, loaded via assets.CommandDesc() and assets.FlagDesc() |
| 101 | + |
| 102 | +**Rationale**: Centralizing user-facing text in a single translatable file prepares for i18n without runtime cost (embedded at compile time) |
| 103 | + |
| 104 | +**Consequence**: System's 30 hidden hook subcommands excluded (not user-facing); flag descriptions use _flags.scope.name convention |
| 105 | + |
| 106 | +--- |
| 107 | + |
| 108 | + |
| 109 | +## Group: Package taxonomy and code placement |
| 110 | + |
| 111 | +## [2026-03-06-200247] cmd/root + core taxonomy for all CLI packages — single-command packages use cmd/root/{cmd.go,run.go}, multi-subcommand packages use cmd/<sub>/{cmd.go,run.go}, shared helpers in core/ |
| 112 | + |
| 113 | +**Status**: Accepted |
| 114 | + |
| 115 | +**Context**: 35 CLI packages had inconsistent flat structures mixing Cmd(), run logic, helpers, and types in the same directory |
| 116 | + |
| 117 | +**Decision**: cmd/root + core taxonomy for all CLI packages — single-command packages use cmd/root/{cmd.go,run.go}, multi-subcommand packages use cmd/<sub>/{cmd.go,run.go}, shared helpers in core/ |
| 118 | + |
| 119 | +**Rationale**: Taxonomical symmetry: every package has the same predictable shape, making navigation instant and agent-friendly |
| 120 | + |
| 121 | +**Consequence**: cmd/ contains only cmd.go + run.go; helpers go to core/; 474 files changed in initial restructuring |
| 122 | + |
| 123 | +--- |
| 124 | + |
| 125 | +## [2026-03-06-200227] Shared entry types and API live in internal/entry, not in CLI packages — domain types that multiple packages consume (mcp, watch, memory) belong in a domain package, not a CLI subpackage |
| 126 | + |
| 127 | +**Status**: Accepted |
| 128 | + |
| 129 | +**Context**: External consumers were importing cli/add for EntryParams/ValidateEntry/WriteEntry, creating a leaky abstraction |
| 130 | + |
| 131 | +**Decision**: Shared entry types and API live in internal/entry, not in CLI packages — domain types that multiple packages consume (mcp, watch, memory) belong in a domain package, not a CLI subpackage |
| 132 | + |
| 133 | +**Rationale**: Domain types in CLI packages force consumers to depend on CLI internals; internal/entry provides a clean boundary |
| 134 | + |
| 135 | +**Consequence**: entry aliases Params from add/core to avoid import cycle (entry imports add/core for insert logic); future work may move insert logic to entry to eliminate the cycle |
| 136 | + |
| 137 | +--- |
| 138 | + |
| 139 | +## [2026-03-13-151954] Templates and user-facing text live in assets, structural constants stay in config |
| 140 | + |
| 141 | +**Status**: Accepted |
| 142 | + |
| 143 | +**Context**: Ongoing refactoring session moving Tpl* constants out of config/ |
| 144 | + |
| 145 | +**Decision**: Templates and user-facing text live in assets, structural constants stay in config |
| 146 | + |
| 147 | +**Rationale**: config/ is for structural constants (paths, limits, regexes); assets/ is for templates, labels, and text that would need i18n. Clean separation of concerns |
| 148 | + |
| 149 | +**Consequence**: All tpl_entry.go, tpl_journal.go, tpl_loop.go, tpl_recall.go moved to assets/ |
| 150 | + |
| 151 | +--- |
| 152 | + |
| 153 | + |
| 154 | +## Group: Eager init over lazy loading |
| 155 | + |
| 156 | +## [2026-03-18-193631] Eager Init() for static embedded data instead of per-accessor sync.Once |
| 157 | + |
| 158 | +**Status**: Accepted |
| 159 | + |
| 160 | +**Context**: 4 sync.Once guards + 4 exported maps + 4 Load functions + a wrapper package for YAML that never mutates. |
| 161 | + |
| 162 | +**Decision**: Eager Init() for static embedded data instead of per-accessor sync.Once |
| 163 | + |
| 164 | +**Rationale**: Data is static and required at startup. sync.Once per accessor is cargo cult. One Init() in main.go is sufficient. Tests call Init() in TestMain. |
| 165 | + |
| 166 | +**Consequence**: Maps unexported, accessors are plain lookups, permissions and stopwords also loaded eagerly. Zero sync.Once remains in the lookup pipeline. |
| 167 | + |
| 168 | +--- |
| 169 | + |
| 170 | +## [2026-03-16-104143] Explicit Init over package-level init() for resource lookup |
| 171 | + |
| 172 | +**Status**: Accepted |
| 173 | + |
| 174 | +**Context**: server/resource package used init() to silently build the URI lookup map |
| 175 | + |
| 176 | +**Decision**: Explicit Init over package-level init() for resource lookup |
| 177 | + |
| 178 | +**Rationale**: Implicit init hides startup dependencies, makes ordering unclear, and is harder to test. Explicit Init called from NewServer makes the dependency visible. |
| 179 | + |
| 180 | +**Consequence**: res.Init() called explicitly from NewServer before ToList(); no package-level side effects |
| 181 | + |
| 182 | +--- |
| 183 | + |
| 184 | + |
| 185 | +## Group: Pure logic separation of concerns |
| 186 | + |
| 187 | +## [2026-03-15-230640] Pure-logic CompactContext with no I/O — callers own file writes and reporting |
| 188 | + |
| 189 | +**Status**: Accepted |
| 190 | + |
| 191 | +**Context**: MCP server and CLI compact command both implemented task compaction independently, with the MCP handler using a local WriteContextFile wrapper |
| 192 | + |
| 193 | +**Decision**: Pure-logic CompactContext with no I/O — callers own file writes and reporting |
| 194 | + |
| 195 | +**Rationale**: Separating pure logic from I/O lets both MCP (JSON-RPC responses) and CLI (cobra cmd.Println) callers control output and file writes. Eliminates duplication and the unnecessary mcp/server/fs package |
| 196 | + |
| 197 | +**Consequence**: tidy.CompactContext returns a CompactResult struct; callers iterate FileUpdates and write them. Archive logic stays in callers since MCP and CLI have different archive policies |
| 198 | + |
| 199 | +--- |
| 200 | + |
| 201 | +## [2026-03-16-122033] Server methods only handle dispatch and I/O, not struct construction |
| 202 | + |
| 203 | +**Status**: Accepted |
| 204 | + |
| 205 | +**Context**: MCP server had ok/error/writeError as methods plus prompt builders that didn't use Server state — they just constructed response structs |
| 206 | + |
| 207 | +**Decision**: Server methods only handle dispatch and I/O, not struct construction |
| 208 | + |
| 209 | +**Rationale**: Methods that don't access receiver state hide their true dependencies and inflate the Server interface. Free functions make the dependency graph explicit and are independently testable. |
| 210 | + |
| 211 | +**Consequence**: New response helpers go in server/out, prompt builders in server/prompt. Server methods are limited to dispatch (handlePromptsGet) and I/O (writeJSON, emitNotification). Same principle applies to future tool/resource builders. |
| 212 | + |
| 213 | +--- |
| 214 | + |
| 215 | +## [2026-03-23-003346] Pure-data param structs in entity — replace function pointers with text keys |
| 216 | + |
| 217 | +**Status**: Accepted |
| 218 | + |
| 219 | +**Context**: MergeParams had UpdateFn callback, DeployParams had ListErr/ReadErr function pointers — both smuggled side effects into data structs |
| 220 | + |
| 221 | +**Decision**: Pure-data param structs in entity — replace function pointers with text keys |
| 222 | + |
| 223 | +**Rationale**: Text keys are pure data, keep entity dependency-free, and the consuming function can do the dispatch itself |
| 224 | + |
| 225 | +**Consequence**: All cross-cutting param structs in entity must be function-pointer-free; I/O functions passed as direct parameters |
| 226 | + |
| 227 | +--- |
| 228 | + |
| 229 | + |
0 commit comments