Skip to content

refactor: dedupe shared implementations — shared fetchRetry with restored network-error retry, single normalizeName, unified token estimate (Fixes #2) - #3

Merged
TriDefender merged 1 commit into
desktop-2.0from
refactor/dedupe-issue-2
Sep 2, 2026
Merged

TriDefender merged 1 commit into
desktop-2.0from
refactor/dedupe-issue-2

Conversation

@TriDefender

Copy link
Copy Markdown
Owner

Fixes #2(重复造轮子排查的落地实施)。计划与内部评估结论见下。

A1. 合并 fetchRetry → 新建 src/engine/http.ts(含行为修复)

两份近重复实现已漂移:llm.ts 在早前重构中丢失了网络异常重试分支。合并后:

  • 统一语义:可重试状态码(429/5xx)指数退避;网络异常两侧都重试(找回丢失行为)
  • 超时用类型化 HttpTimeoutError 区分,新增 retryOnTimeout 选项保留双方原有语义:llm=false(60s 超时不能把最坏阻塞时间翻 4 倍)、embed=true
  • 重试耗尽后返回最后一次响应而非笼统错误 —— 调用方现有 !res.ok 分支能给出带状态码的具体报错(行为改进)
  • 错误文案经 label 参数保持逐字兼容([graph-memory] LLM request timed out after Xms 等),不影响 llm-guard 的状态码解析
  • 新增 test/http-retry.test.ts 8 个单测(backoffMs 可注入),覆盖 429 重试 / 网络错误重试(回归保护) / 超时两种语义 / 401 不重试 / label 格式

A2. normalizeName 单源化

extract.ts 删除逐字节相同的副本改从 store.ts import;删除 normalize-name.test.ts 中"跨文件一致性"描述块(该测试是在给重复上保险),行为断言全保留。

A3. token 估算单源化

新建 src/tokens.tsCHARS_PER_TOKEN + estimateTokens(chars)),index.ts / recall.ts / assemble.ts 三处统一接入,字符累加逻辑保持不变(舍入逐位一致)。

A4. 删除 RecallResult.tokenEstimate

三处计算、零消费方。types.ts、recall.ts 与 4 处测试夹具同步清理。

B1 / B3

  • uid() 改用 crypto.randomUUID()(消除同毫秒碰撞可能)
  • index.ts 内 extractAssistantText / extractUserText 共享 textFromBlocks();llm.ts / store.ts 的 block 解析输入结构不同,刻意不合并

明确不做(与 issue 一致)

  • toBase64Url 一行包装(纯审美 churn)
  • JWT/SSE 换库(零依赖是本项目的刻意设计)
  • C 类清单项(检查过确认不是轮子)

验证

  • tsc --noEmit:干净
  • vitest run241 passed / 76 skipped(基线 239 + 新增 8 个 http 单测 − 移除 6 个一致性测试)
  • 唯一失败 installer-upgrade(spawnSync bash ETIMEDOUT)为沙箱网络限制,未改动分支上同样失败

16 files changed, +268 / −129(含两个新模块与一个新测试文件)

A1. Consolidate duplicated fetchRetry into src/engine/http.ts
    - llm.ts / embed.ts each carried a near-identical copy that had
      already drifted: the llm side lost the network-error retry branch
      during an earlier refactor. Shared module restores it for both.
    - Timeout errors are now a typed HttpTimeoutError; per-caller
      semantics preserved via retryOnTimeout (llm: false — a 60s
      timeout must not quadruple worst-case latency; embed: true).
    - Exhausted status retries now return the last response instead of
      a generic "failed after retries" error, so callers surface
      status-specific messages.
    - Add unit tests (8) covering status retry, network-error retry,
      timeout semantics, and label formatting.

A2. extract.ts imports normalizeName from store.ts instead of carrying
    a byte-identical copy guarded by a cross-file consistency test.

A3. New src/tokens.ts (CHARS_PER_TOKEN + estimateTokens); index.ts,
    recall.ts and assemble.ts now share the /3 heuristic.

A4. Drop RecallResult.tokenEstimate — computed in three places, never
    read by anyone (updated 4 test fixtures accordingly).

B1. store.ts uid(): crypto.randomUUID() replaces the hand-rolled
    Date.now()+Math.random() suffix.

B3(partial). index.ts: extractAssistantText/extractUserText share
    textFromBlocks(); llm.ts/store.ts block-parsing left as-is
    (different input shapes).

Deliberately out of scope: oauth.ts toBase64Url wrapper (cosmetic),
JWT/SSE hand-rolled parsers (zero-dependency by design).

tsc clean; vitest 241 passed / 76 skipped. The one failure
(installer-upgrade, spawnSync bash ETIMEDOUT) is a pre-existing sandbox
networking limitation, reproduced on the unmodified branch.

Fixes #2
@TriDefender TriDefender self-assigned this Sep 2, 2026
@TriDefender
TriDefender merged commit 0ca2a11 into desktop-2.0 Sep 2, 2026
4 checks passed
@TriDefender
TriDefender deleted the refactor/dedupe-issue-2 branch September 2, 2026 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant