From 795ecf67a5b81ee900353171e379dcf36faed65c Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 01:32:21 +0400 Subject: [PATCH] config: phraser block moves to its own file (V-410) Its validate arm goes with it as validatePhraser. The doc drifted on every numeric field. It claimed NGpuLayers defaults to -1, NCtx to 2048 and Timeout to 30s. Nothing in this package defaults any of them: phraser.DefaultConfig sets -1 and has no callers, and cmd/mavend builds a phraser.Config literally. So an omitted n_gpu_layers reaches llama-server as -ngl 0, CPU-only, masked here only because deploy/mavend.json sets 99. Fixed the comment and left the code, and the discrepancy is filed. --- internal/config/config.go | 60 +------------------------- internal/config/phraser.go | 88 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 58 deletions(-) create mode 100644 internal/config/phraser.go diff --git a/internal/config/config.go b/internal/config/config.go index 1ed0667..07800f0 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -415,52 +415,6 @@ type EmailConfig struct { // DefaultEmailTimeout — extraction budget per message. const DefaultEmailTimeout = 2 * time.Minute -// PhraserConfig — the LLM-backed phraser seam. The daemon spawns llama-server -// as a managed subprocess and sends chat-completion requests to phrase nudge -// and reminder messages. nil ⇒ the template-based Stub is used instead. -// -// ModelPath is the only required field. The rest have sensible defaults: -// - BinPath defaults to "llama-server" (found via PATH at spawn time). -// - Listen defaults to "127.0.0.1:0" (random port, read from stderr). -// - NGpuLayers defaults to -1 (max, uses all available GPU layers). -// - NCtx defaults to 2048. -// - Timeout defaults to 30s per request. -type PhraserConfig struct { - ModelPath string `json:"model_path"` - BinPath string `json:"bin_path,omitempty"` - Listen string `json:"listen,omitempty"` - NGpuLayers int `json:"n_gpu_layers,omitempty"` - NCtx int `json:"n_ctx,omitempty"` - Timeout Duration `json:"timeout,omitempty"` - - // CacheRAMMiB bounds llama-server's prompt cache. Omitted ⇒ 512 MiB, which - // is what keeps the resident model near 1 GB of RSS instead of the 7.9 GB - // measured on 2026-08-03. Set it to -1 to pass no flag at all and let the - // server apply its own 8 GiB default. See phraser.Config.CacheRAMMiB. - CacheRAMMiB int `json:"cache_ram_mib,omitempty"` - - // LLMNudges — let the model word nudges again. Off by default: nudges are - // worded from hand-written Russian templates now (the model broke the - // persona and invented units). Chat, query and reminder phrasing always go - // through the model regardless. See phraser.Config.LLMNudges. - LLMNudges bool `json:"llm_nudges,omitempty"` - - // SwapModels — the gguf files the running daemon is allowed to swap to - // without a restart (Vikunja #250). Empty (the default) means the swap - // capability does not exist: ipc.MethodSwapModel answers ErrUnknownMethod, - // exactly like an unconfigured weather or telegram block. - // - // It is an allowlist and not a directory on purpose. The request carries a - // path, and llama-server is started with it as `-m`; anything short of an - // exact match against a list a human wrote in this file would make "swap the - // model" mean "load a file of your choosing off my disk". ModelPath is - // always swappable back to whether or not it is listed. - // - // Paths must be absolute — the daemon's working directory is not the - // operator's, and a relative path here would resolve somewhere surprising. - SwapModels []string `json:"swap_models,omitempty"` -} - // Duration — a time.Duration that round-trips through JSON as a string // ("60s", "5m", "1h30m"). Plain time.Duration marshals as a nanosecond int, // which is unreadable in a config file; this wrapper uses ParseDuration. @@ -630,18 +584,8 @@ func (c *Config) applyDefaults() { } func (c *Config) validate() error { - if c.Phraser != nil { - if c.Phraser.ModelPath == "" { - return errors.New("phraser.model_path is required") - } - // A relative entry in the swap allowlist would resolve against the - // daemon's working directory, so the path a human reads in this file - // would not be the path llama-server is handed. Fail at startup. - for _, m := range c.Phraser.SwapModels { - if !filepath.IsAbs(m) { - return fmt.Errorf("phraser.swap_models: %q must be an absolute path", m) - } - } + if err := c.validatePhraser(); err != nil { + return err } // The update block is validated here even though mavend never acts on it: a // half-written update config that is only noticed by cmd/mavupdate is noticed diff --git a/internal/config/phraser.go b/internal/config/phraser.go new file mode 100644 index 0000000..b884eae --- /dev/null +++ b/internal/config/phraser.go @@ -0,0 +1,88 @@ +package config + +import ( + "errors" + "fmt" + "path/filepath" +) + +// PhraserConfig — the LLM-backed phraser seam. The daemon spawns llama-server +// as a managed subprocess and sends chat-completion requests to phrase nudge +// and reminder messages. nil ⇒ the template-based Stub is used instead. +// +// ModelPath is the only required field. cmd/mavend fills BinPath +// ("llama-server", found via PATH at spawn time) and Listen ("127.0.0.1:0", a +// random port read back off stderr) when they are empty. +// +// Nothing in this package defaults the rest, and an omitted numeric field +// reaches llama-server as a zero. That matters most for NGpuLayers: see below. +type PhraserConfig struct { + ModelPath string `json:"model_path"` + BinPath string `json:"bin_path,omitempty"` + Listen string `json:"listen,omitempty"` + + // NGpuLayers — layers offloaded to the GPU, passed straight through as + // `-ngl`. Omitted ⇒ 0, which is CPU-only inference. + // + // phraser.DefaultConfig says -1 (offload everything), but nothing calls it: + // cmd/mavend builds a phraser.Config literally and copies this field across. + // So the deploy config carries `"n_gpu_layers": 99` and must keep carrying + // it. Vikunja has the discrepancy; do not "fix" it by writing a default + // here, because that would change what a box without the key does. + NGpuLayers int `json:"n_gpu_layers,omitempty"` + + // NCtx — context window, passed through as `-c`. Omitted ⇒ 0, which lets + // llama-server pick. The resident model is a Thinking variant and needs + // 4096; the deploy config sets it. + NCtx int `json:"n_ctx,omitempty"` + + // Timeout — per-request budget. Omitted ⇒ phraser's own default. + Timeout Duration `json:"timeout,omitempty"` + + // CacheRAMMiB bounds llama-server's prompt cache. Omitted ⇒ 512 MiB, which + // is what keeps the resident model near 1 GB of RSS instead of the 7.9 GB + // measured on 2026-08-03. Set it to -1 to pass no flag at all and let the + // server apply its own 8 GiB default. See phraser.Config.CacheRAMMiB. + CacheRAMMiB int `json:"cache_ram_mib,omitempty"` + + // LLMNudges — let the model word nudges again. Off by default: nudges are + // worded from hand-written Russian templates now (the model broke the + // persona and invented units). Chat, query and reminder phrasing always go + // through the model regardless. See phraser.Config.LLMNudges. + LLMNudges bool `json:"llm_nudges,omitempty"` + + // SwapModels — the gguf files the running daemon is allowed to swap to + // without a restart (Vikunja #250). Empty (the default) means the swap + // capability does not exist: ipc.MethodSwapModel answers ErrUnknownMethod, + // exactly like an unconfigured weather or telegram block. + // + // It is an allowlist and not a directory on purpose. The request carries a + // path, and llama-server is started with it as `-m`; anything short of an + // exact match against a list a human wrote in this file would make "swap the + // model" mean "load a file of your choosing off my disk". ModelPath is + // always swappable back to whether or not it is listed. + // + // Paths must be absolute — the daemon's working directory is not the + // operator's, and a relative path here would resolve somewhere surprising. + SwapModels []string `json:"swap_models,omitempty"` +} + +// validatePhraser refuses a block with no model, and a swap allowlist entry +// that would resolve somewhere other than where a reader of this file expects. +func (c *Config) validatePhraser() error { + if c.Phraser == nil { + return nil + } + if c.Phraser.ModelPath == "" { + return errors.New("phraser.model_path is required") + } + // A relative entry in the swap allowlist would resolve against the + // daemon's working directory, so the path a human reads in this file + // would not be the path llama-server is handed. Fail at startup. + for _, m := range c.Phraser.SwapModels { + if !filepath.IsAbs(m) { + return fmt.Errorf("phraser.swap_models: %q must be an absolute path", m) + } + } + return nil +}