diff --git a/internal/config/memory_review.go b/internal/config/memory_review.go index 9eb2f88..ebd93bb 100644 --- a/internal/config/memory_review.go +++ b/internal/config/memory_review.go @@ -12,7 +12,8 @@ import ( // 为什么用独立加载器而不是挂进 Config 根结构：Config 由 Load 统一解码，而 // 本切片刻意**不碰** Config / 记忆整理那组键（并行切片正在改它们）。这里复用 // 与 LoadLLMDefaults 同一手法（parseDotEnv + 同一份 `env:"…"` 解码器）， -// 因此优先级与 Load 逐字一致：进程环境变量 > `.env` > 标签默认值。 +// 因此优先级与 Load 逐字一致：进程环境变量 > `.env` > 标签默认值（TimeoutSec +// 没有标签默认值，见该字段的注释 —— 未配置时它读出 0，由 memory 包定其生效值）。 // envPath 为空表示不看 `.env`（与 Load 的约定一致）。 type MemoryReviewConfig struct { // Enabled 是入库前审查总开关，默认 true。依据：主人 2026-10-05 的裁定 @@ -28,12 +29,36 @@ type MemoryReviewConfig struct { ScanLimit int `env:"MEMORY_REVIEW_SCAN_LIMIT" default:"200"` // TimeoutSec 是单条审查的墙钟上限（秒）。超时 ⇒ **不做任何改写**， // 记忆照旧保留 + WARN（design/23 §15 D3-5 的 fail-safe）。 - TimeoutSec int `env:"MEMORY_REVIEW_TIMEOUT_SEC" default:"20"` + // + // 🔴 **本字段刻意没有 `default:` 标签**（2026-10-13，切片 M16）。单条审查预算的 + // **唯一真源**是 memory 包的 `defaultReviewTimeout`（90s，推导见 + // internal/runtime/memory/preinsert_config.go 的文件头注释）。这里原写 + // `default:"20"`，后果不是「默认值是 20」而是**默认值根本不由 memory 侧决定**： + // env 层永远产出一个非零值 ⇒ memory 侧那道「≤0 才回落默认」的归一化成了**死代码** + // （90s 从未生效，生产启动行 `memory ingest review started … timeout=20s` 即铁证， + // 而实测召回链就要 21~23s ⇒ 每条候选满额的审查都超时、连留痕都不写）。 + // + // 去掉字面默认后的**生效值**（路径：本字段 → cmd/cyrene/wire_memory_review.go 的 + // `ReviewConfig.Timeout` → `normalizeReviewConfig`）： + // + // `.env` / 进程环境没写这个键 ⇒ 这里 0 ⇒ memory 侧回落 90s + // 写了正数 ⇒ 原样生效（归一化只动 ≤0） + // 写了 0 / 负数 ⇒ 回落 90s（与项目其它旋钮同一口径：≤0 = 用默认） + // + // ⚠️ 别在这里重新加回字面默认值 —— 那会让 memory 侧的 90s 再次变成死代码， + // 而 memory 包的 TestReviewTimeoutEffectiveValueIsTheOnlyDefault 会立刻变红 + // （它走完整组装路径取生效值，不比对常量）。 + TimeoutSec int `env:"MEMORY_REVIEW_TIMEOUT_SEC"` } // LoadMemoryReviewConfig 读取入库前审查的旋钮。非法（负数）值不在这里报错， // 而是与项目其它旋钮同一口径 —— **≤0 视为「用默认」**，由 memory 包 normalize // 收敛，避免一个笔误让进程起不来。 +// +// 🔴 因此本函数**对 TimeoutSec 不做任何回落**（切片 M16）：返回 0 就是「没配」， +// 调用方必须原样交给 memory 包的 ReviewConfig（组合根今天就是这么做的）， +// 「0 ⇒ 多少秒」由 `normalizeReviewConfig` 在那**唯一一处**决定。调用方若在这里 +// 又补一个自己的默认值，全链就会重新长出第二个真源 —— 那正是本切片修掉的形态。 func LoadMemoryReviewConfig(envPath string) (MemoryReviewConfig, error) { fromEnv := map[string]string{} if envPath != "" { diff --git a/internal/config/memory_review_test.go b/internal/config/memory_review_test.go index 26508f1..fc8abc7 100644 --- a/internal/config/memory_review_test.go +++ b/internal/config/memory_review_test.go @@ -1,13 +1,47 @@ package config import ( + "os" "testing" ) +// reviewTimeoutEnvUnset 让 MEMORY_REVIEW_TIMEOUT_SEC 在本次用例中**真的未设置**。 +// +// 为什么不用 t.Setenv(key, "")：空串在 decode.go 的 lookupRaw 里**照样命中 env 分支** +// （`os.LookupEnv` 对空值返回 ok=true）⇒ 配置层读出 0，于是**看不见 tag 上的字面 +// 默认值** —— 而那正是本切片的回归面（tag 上再加回 `default:"20"` 必须变红）。 +// 未设置才会走到 tag 默认那一档。 +func reviewTimeoutEnvUnset(t *testing.T) { + t.Helper() + old, had := os.LookupEnv("MEMORY_REVIEW_TIMEOUT_SEC") + t.Cleanup(func() { + if had { + _ = os.Setenv("MEMORY_REVIEW_TIMEOUT_SEC", old) + return + } + _ = os.Unsetenv("MEMORY_REVIEW_TIMEOUT_SEC") + }) + if err := os.Unsetenv("MEMORY_REVIEW_TIMEOUT_SEC"); err != nil { + t.Fatalf("unset MEMORY_REVIEW_TIMEOUT_SEC: %v", err) + } +} + // TestLoadMemoryReviewConfigDefaults 锁定 D3「入库前审查」（design/23 §15.1 D3） // 的 env 契约默认值。它们必须与 memory 包的兜底默认值一致 —— 两处不一致会让 // 「没配这个键」与「配了但解析失败」走向不同的行为。 +// +// 🔴 TimeoutSec 是**例外的那一个，而且是刻意的**（切片 M16，2026-10-13）： +// 它**没有**字面默认值，未配置时就是 0，含义是「没配」而不是「0 秒」。 +// 单条审查预算的唯一真源在 memory 包的 `defaultReviewTimeout`（90s）， +// 全链的生效值 = `ReviewConfig.Timeout` 经 `normalizeReviewConfig` 归一化之后的值。 +// +// 为什么不在这里断言 90：那会让本仓重新长出**两个**真源（env tag 一处、 +// memory 常量一处），而 `.env` 冻结口径（AGENTS §10）本来就把这类「上限/调优」 +// 参数指向**代码默认值**。断言生效值的那条测试在 memory 包 +// （TestReviewTimeoutEffectiveValueIsTheOnlyDefault，走完整组装路径）—— +// 本包不能引 memory 包（memory → config 已成立，反向会成环）。 func TestLoadMemoryReviewConfigDefaults(t *testing.T) { + reviewTimeoutEnvUnset(t) cfg, err := LoadMemoryReviewConfig("") if err != nil { t.Fatalf("LoadMemoryReviewConfig() error: %v", err) @@ -21,13 +55,19 @@ func TestLoadMemoryReviewConfigDefaults(t *testing.T) { if cfg.ScanLimit != 200 { t.Errorf("MEMORY_REVIEW_SCAN_LIMIT = %d, want 200（§15 D3-4 每条审查有界）", cfg.ScanLimit) } - if cfg.TimeoutSec != 20 { - t.Errorf("MEMORY_REVIEW_TIMEOUT_SEC = %d, want 20（§15 D3-5 超时即不改写）", cfg.TimeoutSec) + if cfg.TimeoutSec != 0 { + t.Errorf("MEMORY_REVIEW_TIMEOUT_SEC = %d, want 0（未配置）—— "+ + "这里出现非零值说明 tag 上又被加回了字面默认值，memory 侧的 "+ + "defaultReviewTimeout 会再次变成死代码（M16 修的正是这个形态）", cfg.TimeoutSec) } } // TestLoadMemoryReviewConfigEnvOverrides 锁定键名与类型：写进 .env 的旋钮真的 // 生效（键名写错会静默沿用默认值，这类错误只能靠这条用例抓住）。 +// +// TimeoutSec 另钉一条口径（M16）：**正数原样透传、≤0 原样透传**。配置层不做 +// 「坏值回落」——「0 或负数 ⇒ 用默认 90s」是 memory 包 normalizeReviewConfig +// 的语义，全链只有那一处决定它（组合根不许在这里补默认值）。 func TestLoadMemoryReviewConfigEnvOverrides(t *testing.T) { t.Setenv("MEMORY_REVIEW_ENABLED", "false") t.Setenv("MEMORY_REVIEW_QUEUE_SIZE", "8") @@ -41,4 +81,22 @@ func TestLoadMemoryReviewConfigEnvOverrides(t *testing.T) { if cfg.Enabled || cfg.QueueSize != 8 || cfg.ScanLimit != 32 || cfg.TimeoutSec != 3 { t.Fatalf("MEMORY_REVIEW_* 未生效：%+v", cfg) } + + for _, tc := range []struct { + raw string + want int + }{ + {"0", 0}, // 显式 0 = 用默认（下游归一化），不许在这里被改成 90 + {"-5", -5}, // 负数同样是「用默认」，也不在这里报错（AGENTS：不让笔误把进程弄起不来） + } { + t.Setenv("MEMORY_REVIEW_TIMEOUT_SEC", tc.raw) + got, err := LoadMemoryReviewConfig("") + if err != nil { + t.Fatalf("LoadMemoryReviewConfig(%q) error: %v", tc.raw, err) + } + if got.TimeoutSec != tc.want { + t.Fatalf("MEMORY_REVIEW_TIMEOUT_SEC=%q 读出 %d, want %d（配置层不回落，回落只在 memory 包）", + tc.raw, got.TimeoutSec, tc.want) + } + } } diff --git a/internal/runtime/memory/preinsert_config.go b/internal/runtime/memory/preinsert_config.go index e1b23ce..7eb68b2 100644 --- a/internal/runtime/memory/preinsert_config.go +++ b/internal/runtime/memory/preinsert_config.go @@ -18,9 +18,18 @@ const ( defaultReviewQueueSize = 256 // defaultReviewScanLimit 是每次审查在同一分区内最多比对的既有条目数。 defaultReviewScanLimit = 200 - // defaultReviewTimeout 是单条审查的墙钟上限。 + // defaultReviewTimeout 是单条审查的墙钟上限，**也是全链的唯一真源** + // （切片 M16，2026-10-13）：配置层 `config.MemoryReviewConfig.TimeoutSec` + // 的 env tag **刻意不带** `default:`，没配时读出 0（= 没配）， + // 由本文件下方的 normalizeReviewConfig 落地成这个常量。 // - // 🔴 2026-10-13 由 20s 抬到 90s。依据是**实测的链长**，不是感觉： + // 🔴 为什么要把真源搬到这里：原先 tag 上写着 `default:"20"`，而 20s 又等于 + // 当时的本常量 ⇒ 看着「两处一致」，实际是 **env 层永远产出一个非零值**， + // 「≤0 才回落默认」这条归一化**永远是死代码** ⇒ 本常量改成 90s 后生产启动行 + // 仍是 `timeout=20s`（塔在部署后读真实启动行抓到）。所以别再往 env tag 上写 + // 字面默认值：那等于把「默认值」搬回配置层，本常量又一次变成装饰。 + // + // 取值推导如下（2026-10-13 由 20s 抬到 90s），依据是**实测的链长**，不是感觉： // 单条审查的召回链是「新条 1 + ScanLimit 200 = 201 条文本」，而 embedding 侧 // 一次请求只吃 DefaultBatchLimit 条（改前 4、改后 8，见 embedding/budget.go 的 // 文件头实测）⇒ 链长 = ⌈201/批次⌉ 个请求： @@ -162,6 +171,9 @@ func normalizeReviewConfig(cfg ReviewConfig) ReviewConfig { cfg.ScanLimit = defaultReviewScanLimit } if cfg.Timeout <= 0 { + // 单条审查预算的**唯一**落地处（切片 M16）：配置层的 env tag 不带默认值， + // 所以「没配」与「配了 ≤0」都走这一行。全链的生效值就是这一行的结果 —— + // 启动行 `memory ingest review started … timeout=` 打印的也正是它。 cfg.Timeout = defaultReviewTimeout } if cfg.Threshold <= 0 { diff --git a/internal/runtime/memory/preinsert_recall_test.go b/internal/runtime/memory/preinsert_recall_test.go index 79f19fd..9639c26 100644 --- a/internal/runtime/memory/preinsert_recall_test.go +++ b/internal/runtime/memory/preinsert_recall_test.go @@ -1,9 +1,15 @@ package memory import ( + "bytes" + "context" + "log/slog" + "os" + "strings" "testing" "time" + "git.yeij.top/AskaEth/Cyrene_v2/internal/config" "git.yeij.top/AskaEth/Cyrene_v2/internal/runtime/embedding" ) @@ -26,6 +32,29 @@ const ( measuredReviewChainLoaded = 57 * time.Second ) +// reviewTimeoutEnvUnset 让 MEMORY_REVIEW_TIMEOUT_SEC 在本次用例中**真的未设置**。 +// +// 为什么不用 t.Setenv(key, "")：空串在 decode.go 的 lookupRaw 里**照样命中 env +// 分支**（`os.LookupEnv` 对空值返回 ok=true）⇒ 配置层读出 0，于是**看不见 env tag +// 上的字面默认值** —— 而那正是本切片的回归面（tag 上再加回 `default:"20"` 必须 +// 让下面的生效值用例变红）。只有「未设置」才会走到 tag 默认那一档。 +// （用 os.Unsetenv 而不是 t.Setenv 是刻意的：Go 没有「置回未设置」的测试 API， +// 因此这里自己存旧值 + 在 t.Cleanup 里还原。本用例不并行，故无竞争。） +func reviewTimeoutEnvUnset(t *testing.T) { + t.Helper() + old, had := os.LookupEnv("MEMORY_REVIEW_TIMEOUT_SEC") + t.Cleanup(func() { + if had { + _ = os.Setenv("MEMORY_REVIEW_TIMEOUT_SEC", old) + return + } + _ = os.Unsetenv("MEMORY_REVIEW_TIMEOUT_SEC") + }) + if err := os.Unsetenv("MEMORY_REVIEW_TIMEOUT_SEC"); err != nil { + t.Fatalf("unset MEMORY_REVIEW_TIMEOUT_SEC: %v", err) + } +} + // TestEmbeddingTimeoutFitsReviewBudget 钉住两条**跨包**口径： // // 1. embedding 单次调用的独立超时必须严格小于入库前审查的单条总预算； @@ -43,6 +72,9 @@ const ( // 预算吃掉都会被它抓到。旧值 20s 正是踩在这条线上的：实测链长 21~23s（空闲）， // 于是每一条候选满额的审查都必然超时，7 天里 2274 条审查因此降级为空 recall_mode。 // +// ⚠️ 本条比的是**本包的常量**，因此**看不见跨包漂移**（env 层另有一个默认值时它 +// 照样绿）—— 那条防线在下面的 TestReviewTimeoutEffectiveValueIsTheOnlyDefault。 +// // embedding 侧同一口径也有一条钉值测试（api_url_test.go），两边互为护栏。 func TestEmbeddingTimeoutFitsReviewBudget(t *testing.T) { t.Parallel() @@ -68,3 +100,127 @@ func TestEmbeddingTimeoutFitsReviewBudget(t *testing.T) { measuredReviewChainIdle, defaultReviewTimeout) } } + +// TestReviewTimeoutExplicitEnvValueWins 钉住另一半口径（M16）：`.env` / 进程环境 +// **显式写了**这个键时，它必须**原样到达生效值**（不被 memory 侧的默认值顶掉）。 +// +// 与 TestReviewTimeoutEffectiveValueIsTheOnlyDefault 合起来覆盖两个情形： +// +// 没写 ⇒ 生效值 = 90s（唯一真源） +// 写了 45 ⇒ 生效值 = 45s（显式优先，且**不做任何「保底抬到 90」的偷偷加码**） +// +// 后者同样要钉：运维为了让某台机器更早放弃而调低预算，是**显式选择**， +// 代码不许自作主张改回去（要改回去也该改配置，而不是让配置失效）。 +func TestReviewTimeoutExplicitEnvValueWins(t *testing.T) { + t.Setenv("MEMORY_REVIEW_TIMEOUT_SEC", "45") + + cfg, err := config.LoadMemoryReviewConfig("") + if err != nil { + t.Fatalf("LoadMemoryReviewConfig: %v", err) + } + if cfg.TimeoutSec != 45 { + t.Fatalf("env 层的 MEMORY_REVIEW_TIMEOUT_SEC = %d, want 45", cfg.TimeoutSec) + } + reviewer, err := NewPreInsertReviewer(ReviewConfig{ + Enabled: true, + Threshold: 0.8, + Timeout: time.Duration(cfg.TimeoutSec) * time.Second, + }, ReviewDeps{Store: newTestStore(t), Log: testLogger()}) + if err != nil { + t.Fatalf("NewPreInsertReviewer: %v", err) + } + if got := reviewer.cfg.Timeout; got != 45*time.Second { + t.Fatalf("显式配置的 45s 没到达生效值：%s", got) + } +} + +// TestReviewTimeoutEffectiveValueIsTheOnlyDefault 是**全链生效值**的钉值测试 +// （切片 M16，2026-10-13）。它刻意走**完整组装路径**，一层都不绕过： +// +// config.LoadMemoryReviewConfig("") ← env 层（"env 档没写这个键" 的形态） +// → ReviewConfig{Timeout: 秒 × time.Second} ← 组合根 cmd/cyrene 那一行 +// → NewPreInsertReviewer → normalizeReviewConfig ← 唯一落地处 +// → r.cfg.Timeout（= 启动行打印的 timeout=…，塔复验读的就是它） +// +// 🔴 它为什么必须存在（而 TestEmbeddingTimeoutFitsReviewBudget 拦不住这条）： +// 那一条比的是**本包的常量** `defaultReviewTimeout`，它从来看不见 env 层。 +// 而线上坏掉的形态恰好发生在**另一个包**：config 侧的 env tag 带着 +// `default:"20"` ⇒ env 层永远返回 20 ⇒ 本包「≤0 才回落默认」的归一化成了**死代码** +// ⇒ 常量的 90s **一次也没生效过**。两处都是「90 > 实测链长 + T1」的绿色断言， +// 生产启动行却是 `timeout=20s` —— 常量比对与生效值比对是**两件事**： +// +// 比常量 = 「我打算用多少」（单包自洽，跨包漂移看不见） +// 比生效值 = 「进程实际会拿到多少」（跨包漂移立刻见红） +// +// 后者才拦得住「同一个值两处副本、只有一处被改」（本仓 `reviewBudgetRef` 是前科）。 +// 变异自证：把 config 侧 tag 的 `default:"20"` 加回去 ⇒ 本用例必红（下方 +// cfg.TimeoutSec != 0 那条会先报出来，随后生效值 20s 也会小于下界）。 +// +// 不许并行：下面要把 env 档置成**真的未设置**（见 reviewTimeoutEnvUnset 的理由）。 +func TestReviewTimeoutEffectiveValueIsTheOnlyDefault(t *testing.T) { + reviewTimeoutEnvUnset(t) + + cfg, err := config.LoadMemoryReviewConfig("") + if err != nil { + t.Fatalf("LoadMemoryReviewConfig: %v", err) + } + if cfg.TimeoutSec != 0 { + // 用 Errorf 而不是 Fatalf：这样**两条防线都会报出来** —— 「配置层又长了 + // 一个字面默认值」与「组装后的生效值盖不住实测链长」。变异成修前形态时 + // 两条同时见红，读的人不必猜是哪一层坏掉。 + t.Errorf("env 层的 MEMORY_REVIEW_TIMEOUT_SEC = %d, want 0（未配置）—— "+ + "非零意味着配置层又长出了字面默认值，memory 侧的 %s 会再次成为死代码（M16 的形态）", + cfg.TimeoutSec, defaultReviewTimeout) + } + + // 逐字复刻组合根把配置层搬进 ReviewConfig 的那一行 + // （cmd/cyrene/wire_memory_review.go：`Timeout: time.Duration(cfg.TimeoutSec) * time.Second`）。 + var startupLine bytes.Buffer + reviewer, err := NewPreInsertReviewer(ReviewConfig{ + Enabled: true, + Threshold: 0.8, + Timeout: time.Duration(cfg.TimeoutSec) * time.Second, + }, ReviewDeps{ + Store: newTestStore(t), + Log: slog.New(slog.NewTextHandler(&startupLine, nil)), + }) + if err != nil { + t.Fatalf("NewPreInsertReviewer: %v", err) + } + + effective := reviewer.cfg.Timeout + // 生效值证据：塔重启后拿它对照生产启动行 + // `msg="memory ingest review started" … timeout=…`。 + t.Logf("组装后 timeout = %s（env 层未配置；启动行会原样打印它）", effective) + + worst := measuredReviewChainLoaded + defaultT1Timeout + if effective != defaultReviewTimeout { + // 同样是 Errorf：与上面那条一起定位「哪一层把默认值改掉了」。 + t.Errorf("生效值 %s != 唯一真源 defaultReviewTimeout %s（配置层与 memory 层漂移了）", + effective, defaultReviewTimeout) + } + if effective <= worst { + t.Fatalf("组装后的单条审查预算 %s 必须严格大于「实测最坏召回链 %s + T1 子超时 %s = %s」—— "+ + "否则链还没跑完预算就用光（生产 20s 时 7 天里 2274 条审查因此降级为空 recall_mode）", + effective, measuredReviewChainLoaded, defaultT1Timeout, worst) + } + if measuredReviewChainIdle >= effective { + t.Fatalf("空闲实测链长 %s 已不小于生效预算 %s（余量必须为正）", + measuredReviewChainIdle, effective) + } + + // 启动行是塔唯一的复验面：它必须真的打印这个生效值。把 Start/Stop 跑起来， + // 因为「日志里写的是 r.cfg.Timeout 还是另一个副本」本身就是要钉住的事。 + ctx, cancel := context.WithCancel(context.Background()) + reviewer.Start(ctx) + stopCtx, stopCancel := context.WithTimeout(context.Background(), 5*time.Second) + defer stopCancel() + if err := reviewer.Stop(stopCtx); err != nil { + t.Fatalf("Stop: %v", err) + } + cancel() + if line := startupLine.String(); !strings.Contains(line, "memory ingest review started") || + !strings.Contains(line, "timeout="+effective.String()) { + t.Fatalf("启动行没有打印生效值 timeout=%s：%q", effective, line) + } +}