Skip to content

fix(copilot): align overage fallback with parser shape split - #16

Merged
linletian merged 1 commit into
mainfrom
fix/copilot-overage-fallback-double-count
Aug 2, 2026
Merged

fix(copilot): align overage fallback with parser shape split#16
linletian merged 1 commit into
mainfrom
fix/copilot-overage-fallback-double-count

Conversation

@linletian

@linletian linletian commented Aug 2, 2026

Copy link
Copy Markdown
Owner

背景

PR #14 合并后,PR #15 的评审发现两处实现与 docs/copilot-overage-stuck-at-100-percent.md §7 策略不一致,其中 RefreshService fallback 双计已由 PR #15f571dea)修复。本 PR 收敛为仅剩的净增量:Parser legacy 分支的 - remaining 修复。

2026-08-02 已 rebase 到 main(5586320,含 PR #15 合并)。原分支含的 RefreshService fallback 分流、误导注释删除、Service 层重复测试,均与 PR #15 内容重复,冲突解决时已取 main 侧,不在本 PR 范围内。详见 §10.1 更新后的归属说明。

改动

文件 内容
CopilotResponseParser.swift legacy fallback 恢复 entitlement - remaining + overageCount6a85578 误改为 entitlement + overageCount,仅在 remaining == 0 时等价)
CopilotResponseParserTests.swift 新增 testLegacyOverageWithPositiveRemainingSubtractsRemaining(100/30/5 → 75%,旧公式会算 105% 高估)
docs/copilot-overage-stuck-at-100-percent.md §5/§7/§8/§9 公式描述同步;§10.1 修正为两分支的准确归属
docs/provider-interfaces/copilot.md overage 分流公式描述同步

风险

  • legacy 形状下 remaining 通常被夹到 0,此修复在 remaining == 0 时与旧公式完全同解,仅防御 remaining > 0 且 overage 的边界(成本为零)
  • 不改成功路径、不改 parser 对外契约,仅公式一行

验证

@linletian linletian left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

评审:与 PR15 高度重复,净增量仅 Parser legacy 修复

背景

本分支基于 580bc73#14 合并后),创建于 PR15 合并之前。它是 PR15 评审意见(07-31 #1 双计 bug + #7 parser legacy 丢 - remaining)的同题独立实现——当时作者尚不知道 PR15 分支会一并修复。PR15 现已合并进 main。

✅ 改动正确性(PR16 自身)

  1. RefreshService fallback 分流remaining < 0 ? entitlement - remaining : max(0, entitlement - remaining) + overageCount —— 正确,与已合并的 f571dea 公式逐字一致(两分支独立达成相同结论)
  2. Parser legacy 恢复 - remainingentitlement - remaining + overageCount):本 PR 的净增量,采纳 07-31 评审 #7remaining > 0 且 overage 时正确(fixture 100/30/5 → 75%,旧公式会算 105% 高估);remaining == 0 时与旧公式等价、零成本
  3. 误导注释删除:"NOT a provider-agnostic extension point" 直接回应评审 #5,质量好
  4. 测试testLegacyOverageWithPositiveRemainingSubtractsRemaining(parser)与 testCopilotOverageFallbackWithoutCreditsUsedDoesNotDoubleCount(service)均正确且通过(本分支实测:358 个测试中仅 Kimi 的 testUnparseableWeeklyResetTimeDoesNotDeclareFiveHourPolicy 失败——那是 PR15 6bae5d3 已修的预存在问题,本分支基线未包含)
  5. 文档:§5/§7/§8/§9 + provider-interfaces 同步更新到位

🔴 问题(合并前必须处理)

1. 与 main 冲突是真实的,且解决方向不能错

用正确基线(main=5586320)跑 merge-tree 模拟,2 个冲突文件:

  • RefreshService.swift:冲突仅在注释与哨兵写法,核心公式两侧已一致。main 侧含评审 #4Int? 哨兵 + 契约注释(cadab120630250 吸收);本分支仍是 ?? "0") ?? 0 数值守卫。冲突解决必须取 main 侧——两边都能编译,错误选择会静默回退评审 #4 的成果
  • RefreshServiceMappingTests.swifttestCopilotOverageWithoutCreditsUsed(main)vs testCopilotOverageFallbackWithoutCreditsUsedDoesNotDoubleCount(本分支)—— 断言完全相同的重复测试,二选一即可

2. 文档 §10.1 描述过时

§10.1 声称两项修复(RefreshService fallback + parser legacy)都由本分支完成。实际 RefreshService fallback 已由 PR15/f571dea 修复。rebase 后需改写:本分支只贡献 parser legacy 一项。

3. 必须 rebase/merge main

分支缺 PR15 的 6bae5d3 fixture 修复,当前基线测试集是红的(1 失败)。直接合并会丢失 PR15 全部成果。

🟡 建议

  • rebase main 后,RefreshService.swift 冲突取 main 侧(保留 Int? + 契约注释);本分支的 "NOT an extension point" 注释要点已由 main 侧契约注释覆盖,无需保留双份
  • service 层重复测试保留一个
  • Parser legacy 修复 + 测试 + 文档是干净的净增量,自动合并无冲突(merge-tree 验证 4 个文件 merged 干净)

结论

Request changes:方向正确、修复正确,但 90% 内容与已合并的 PR15 重复。合并路径:rebase main → 冲突取 main 侧 → 仅保留 Parser legacy 净增量 → 修正 §10.1。或者更简单:关闭本 PR,把 Parser legacy 修复作为独立小 commit 提到新 PR。

Post-merge review (PR #15) found two divergences from the strategy in
docs/copilot-overage-stuck-at-100-percent.md §7:

- RefreshService's fallback kept `max(0, entitlement - remaining) +
  overageCount` unconditionally, double-counting overage in the new
  API shape when `credits_used` is absent (panel text 120/100 vs the
  parser's 110% bar). Now mirrors the parser: `remaining < 0` ⇒
  `entitlement - remaining`, else add `overageCount`.
- Parser legacy branch dropped `- remaining` in 6a85578; restored
  (equivalent at remaining == 0, correct when > 0).

Also replace the misleading comment claiming the numeric guard is a
provider-agnostic extension point — the block sits inside the
githubCopilot branch, no other supplier can reach it.

Tests: testCopilotOverageFallbackWithoutCreditsUsedDoesNotDoubleCount,
testLegacyOverageWithPositiveRemainingSubtractsRemaining.
@linletian
linletian force-pushed the fix/copilot-overage-fallback-double-count branch from c41e882 to 7b8474f Compare August 2, 2026 10:00
@linletian
linletian merged commit 9cee088 into main Aug 2, 2026
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