mavend: group the recall fields into one wiring struct (V-433)
The review comment asked for basic DI. The answer is the idiom voice.go already had for capabilities — a cohesive *Wiring struct — applied to a group that is not a capability toggle, plus the decision written down so it is a rule and not a habit. recallWiring holds the embedder, the vector store, the personal boundary and the two numbers that gate an answer. They sat in three places on reactiveHandler, with the gate numbers a hundred lines from the store they gate. Its zero value means no recall, so it is a value, not a pointer like the optional-capability groups. dataStore stays out of it. patterns.go, ecosystem_acts.go and confirm.go use it, so it is not part of this cluster. docs/handler-wiring.md records the choice, rejects a container or a wire-style generator outright, defers narrow per-handler interfaces to the package split that would justify them, and states the constraint the task named: a wiring change does not ride a feature PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,62 @@
|
||||
# How reactiveHandler is wired
|
||||
|
||||
*Last verified: 2026-08-04 @ b6abb19. Living doc: correct it in place, do not append.*
|
||||
|
||||
The decision Vikunja #433 asked for, and the rule that follows from it.
|
||||
|
||||
## The decision
|
||||
|
||||
**Group the fields into cohesive wiring structs. No container, no generator, no
|
||||
framework.** `reactiveHandler` (`cmd/mavend/voice.go`) stays the one type the voice
|
||||
server talks to. What changes is that a capability arrives as one named group, not as
|
||||
four more loose fields on a struct that already had thirty.
|
||||
|
||||
The pattern was already in the file before this was written down: `searchWiring`,
|
||||
`kiwixWiring`, `homeWiring`, `netWiring` and `ecosystemWiring` are all this shape, each
|
||||
`nil` when the capability is off. #433 makes it the rule rather than a habit, and adds
|
||||
the case the habit had missed — a group that is not a capability toggle.
|
||||
|
||||
## The worked example
|
||||
|
||||
`recallWiring` (`cmd/mavend/recall.go`) holds the five things the recall path needs:
|
||||
the embedder, the vector store, the personal boundary, and the score and margin that
|
||||
gate an answer. They used to sit in three separate places on the handler with the two
|
||||
gate numbers a hundred lines away from the store they gate.
|
||||
|
||||
Its zero value means "no recall", which is why it is a value and not a pointer. The
|
||||
`*Wiring` types that model an optional capability stay pointers, because `nil` is how
|
||||
"not configured" is spelled and a zero-valued search client would be a client pointed at
|
||||
nothing.
|
||||
|
||||
## Why not the alternatives
|
||||
|
||||
**Narrow consumer-side interfaces at each handler** is the more idiomatic Go answer and
|
||||
it is not rejected, only deferred. It is the right move at the point a handler is pulled
|
||||
into its own package, because that is when the import direction starts to matter. Doing
|
||||
it first would mean writing an interface per handler against a struct nobody can pass
|
||||
anywhere, which is churn bought against a package split that has not happened.
|
||||
|
||||
**A container or a wire-style generator** is rejected outright. This is one binary with
|
||||
one composition root (`wireVoice` in `cmd/mavend/voicewire.go`). Generated wiring would
|
||||
add a build step and a layer of indirection to solve a problem that is currently one
|
||||
composite literal long, and it would make the "is this capability configured" question
|
||||
harder to answer by reading, which is the question this file is mostly about.
|
||||
|
||||
## What this unlocks
|
||||
|
||||
The reason `cmd/mavend/` cannot split into `mavend/actions/` today is that every action
|
||||
handler is a method on a struct with thirty unexported fields: moving handlers to a
|
||||
subdirectory means exporting all of them or inventing an interface to pass through. That
|
||||
was the answer given on the tick.go and voice.go splits, and it is still true. Grouping
|
||||
is the step that makes it false later — a handler that takes `recallWiring` and nothing
|
||||
else can move without the other twenty-five fields following it.
|
||||
|
||||
## The rule
|
||||
|
||||
**A wiring change does not ride a feature PR.** The voice.go and tick.go splits were
|
||||
safe to merge because the moved code diffed identical, line for line. A regrouping that
|
||||
touches the confirm gate or the act allowlist is its own change, reviewed on its own, or
|
||||
it is not reviewable at all.
|
||||
|
||||
New capability, new group. A capability that adds four fields to `reactiveHandler`
|
||||
instead of one struct is the thing this decision exists to stop.
|
||||
Reference in New Issue
Block a user