Skip to content

ci: 新增 data_version 同步 bump 檢查#234

Open
GJRobert wants to merge 2 commits into
mainfrom
claude/main-sandhi-ruby-display-1vo3rp
Open

ci: 新增 data_version 同步 bump 檢查#234
GJRobert wants to merge 2 commits into
mainfrom
claude/main-sandhi-ruby-display-1vo3rp

Conversation

@GJRobert

Copy link
Copy Markdown
Collaborator

資料檔(sandhi_rules.json、reverse_tone_mapping.json、tone_mapping.json、
NAmedias.json、exclusions.json、data/cert|gip/*.json)異動時,若沒有同步
bump data/data_version.json,使用者端 IndexedDB 版本比對會誤判為已是最新,
不會重抓資料(含 sandhi 規則),只能靠 ?force-refresh=true 手動繞過。
過去純靠 commit message 自律(曾被坑兩次),現在改由 CI 擋。

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01BoNfZkBTnuRzCTX6iatqdL

資料檔(sandhi_rules.json、reverse_tone_mapping.json、tone_mapping.json、
NAmedias.json、exclusions.json、data/cert|gip/*.json)異動時,若沒有同步
bump data/data_version.json,使用者端 IndexedDB 版本比對會誤判為已是最新,
不會重抓資料(含 sandhi 規則),只能靠 ?force-refresh=true 手動繞過。
過去純靠 commit message 自律(曾被坑兩次),現在改由 CI 擋。

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BoNfZkBTnuRzCTX6iatqdL
@gemini-code-assist

Copy link
Copy Markdown

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown

Review: ci: 新增 data_version 同步 bump 檢查 (#234)

這個 PR 做了什麼

新增 .github/workflows/data-version-check.yml:當 data/cert/**.jsondata/gip/**.jsontone_mapping.jsonreverse_tone_mapping.jsonsandhi_rules.jsonNAmedias.jsonexclusions.json 這些資料檔有變更、但 data/data_version.json 沒有同步 bump 時,讓 CI fail,防止使用者端 IndexedDB 版本比對誤判為最新而不重抓資料。動機(過去踩過兩次坑、只能靠 ?force-refresh=true 補救)寫得很清楚,也對齊 AGENTS.md 中「版本控制」段落描述的架構。

單一檔案、純新增,改動範圍小、風險低。以下是幾點建議。

正確性 / 潛在問題

  1. pull_request 事件用兩點 diff(base.sha vs head.sha),建議改三點 diff(merge-base)
    目前:

    BASE="${{ github.event.pull_request.base.sha }}"
    HEAD="${{ github.event.pull_request.head.sha }}"
    ...
    CHANGED=$(git diff --name-only "$BASE" "$HEAD")

    base.sha 是 PR 開啟/最後一次 synchronize 當下 base 分支的 commit,不是 merge-base。如果 PR 分支落後 main(沒有 rebase),main 之後對 data_version.json 或其他資料檔的異動也會被算進這次 diff,可能造成誤判(明明這個 PR 沒動資料檔,卻因為 base 分支後來的異動被 fail,或反過來因為 base 分支剛好也 bump 過版本而放過本該擋下的變更)。建議改成三點語法,只看這個 PR 分支自己新增的 commit:

    CHANGED=$(git diff --name-only "$BASE...$HEAD")
  2. 新分支多筆 commit 一次 push 時,fallback 只比對到最後一筆 commit

    if [ -z "$BASE" ] || [[ "$BASE" =~ ^0+$ ]]; then
      BASE="${HEAD}^"
    fi

    這個 fallback 註解說是給「push 到新分支」情境用,但 HEAD^ 只會抓「HEAD 相對於它的直接 parent」的差異,如果一次 push 把好幾個 commit 一起推上新分支,前面幾個 commit 裡對資料檔的異動不會被看到。極端情況(例如整包歷史第一個 commit,HEAD^ 不存在)也會讓 git diff 直接失敗。目前規模下機率低,但值得留意;比較穩妥的作法是抓這次 push 實際的 commit 範圍(例如用 git rev-list 或直接跟遠端 main 的 merge-base 比)。

風格 / 一致性(小建議)

  1. workflow 檔案自己沒有列進 paths 觸發條件
    對照專案裡既有的 sandhi-test.yml,它的 paths 清單有把自己 (.github/workflows/sandhi-test.yml) 加進去,這樣以後改動這個 workflow 的邏輯時,改動本身也會被驗證跑一次。新的 data-version-check.yml 沒有這麼做,之後如果調整比對邏輯(例如上面兩點的修正)不會觸發自我驗證,建議比照加進去。

其他

  • 錯誤訊息(::error::...)清楚說明了為什麼會 fail、以及對應到哪個已知的坑(force-refresh),對之後維護者/PR 作者很友善,這點做得不錯。
  • fetch-depth: 0 是必要的(否則 shallow clone 抓不到 BASE commit),這裡處理對了。
  • 沒有新增自動化測試,但這是 CI workflow 本身,用實際 PR/push 觸發驗證即符合預期,不需要額外測試檔。

整體來說方向正確、能解決文中描述的實際問題,主要建議是把 (1) 兩點/三點 diff 的差異修正,避免長壽命或落後 rebase 的 PR 誤判。

依 PR #234 review 建議修正:
- pull_request 改三點 diff(merge-base),避免落後 rebase 的 PR
  被 base 分支後續異動誤判
- push 到新分支的 fallback 改用跟預設分支的 merge-base,涵蓋
  一次 push 多筆 commit 的情況(原本 HEAD^ 只看得到最後一筆)
- 把 workflow 檔自己加進 paths,比照 sandhi-test.yml 慣例

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BoNfZkBTnuRzCTX6iatqdL
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 18, 2026

Copy link
Copy Markdown

Deploying hakspring with  Cloudflare Pages  Cloudflare Pages

Latest commit: cff3877
Status: ✅  Deploy successful!
Preview URL: https://0eb79018.hakspring.pages.dev
Branch Preview URL: https://claude-main-sandhi-ruby-disp.hakspring.pages.dev

View logs

@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown

Review: ci: 新增 data_version 同步 bump 檢查 (#234)

概述

新增一支 GitHub Actions workflow(.github/workflows/data-version-check.yml),在資料檔(data/cert/**.jsondata/gip/**.jsontone_mapping.jsonreverse_tone_mapping.jsonsandhi_rules.jsonNAmedias.jsonexclusions.json)變更但 data/data_version.json 沒有同步變更時,讓 CI 失敗擋下 PR/push。動機清楚(PR 說明提到過去純靠自律曾出過兩次包),純新增檔案、不影響既有程式邏輯,風險很低。

優點

  • pull_request 事件用三點 diff(BASE...HEAD,即 merge-base diff),可正確排除 base 分支後續異動、避免落後 rebase 的長壽命 PR 誤判,考慮得很周到。
  • push 事件對「新分支、before 為全 0」的情況做了 fallback(用與預設分支的 merge-base 當起點),涵蓋分支上所有 commit,這個邊界情況容易被忽略但這裡有處理到。
  • 註解(中文)清楚解釋每段邏輯的「為什麼」,尤其是三點/兩點 diff 的取捨,可讀性佳。
  • 觸發路徑與倉庫內實際檔案位置(root 下的 tone_mapping.json 等、data/cert/data/gip/)一致,fetch-depth: 0 確保 diff 所需的歷史都在。

可能的問題 / 建議

  1. 檢查只看「檔案是否出現在 diff 裡」,不驗證版本值是否真的變大data-version-check.yml 的 63-69 行):目前邏輯只要 data/data_version.json 這個路徑出現在變更清單就視為「已同步 bump」,即使該次改動只是格式調整、加空白、或版本號被改小/改成跟舊值一樣,檢查依然會通過。既然這支 workflow 的目的就是「確保版本真的有 bump」,建議額外比較 git show "$BASE":data/data_version.jsonHEAD 版本的 version 欄位,確認數值確實有遞增,而不只是檔案路徑被觸碰過。這是目前實作與 PR 目標之間最主要的落差。
  2. 缺少 permissions 區塊:repo 內其他 workflow(claude.ymlclaude-code-review.yml)都有明確宣告 permissions:,這支只做 checkout + shell 檢查、不需要寫入權限,建議加上 permissions: contents: read,符合最小權限原則並與現有慣例一致。
  3. push 事件用兩點 diff(BASE..HEAD)搭配 before/after:一般情況沒問題,但若該次 push 是 force-push/history rewrite,before 可能不是 after 的祖先,兩點 diff 仍會跑,只是語意上未必是「這次新增的內容」。屬於邊界情況,目前不影響核心目的,可以不用現在處理,但值得留意。
  4. 兩處 paths: 清單重複pushpull_request 各寫一份一樣的路徑列表):可以考慮用 YAML anchor(&/*)在同一份 workflow 檔內共用這份清單,減少之後新增資料檔類型時要改兩個地方、容易漏改其中一處的風險。非必要,但是個小的可維護性改善。

安全性

  • Script 中所有 ${{ }} 內插值(repository.default_branchevent_namepull_request.base/head.shaevent.before/after)都是 SHA 或系統控制的固定值,非使用者可自由輸入的文字欄位(如 PR title),script injection 風險低。若要更嚴謹,可以改用 env: 傳遞後在 shell 內用 "$VAR" 引用,是更保守的通用寫法,但這裡不算急迫。

測試覆蓋

  • 這是 CI workflow 本身,沒有(也難以有)傳統單元測試;比較務實的驗證方式是實際跑一次 PR/push 情境(例如刻意只改 sandhi_rules.json 不 bump 版本),確認 workflow 真的會 fail,之後再修正驗證。建議在合併前手動驗證三種情境:純資料檔變更未 bump(應該失敗)、資料檔+版本檔都變更(應該通過)、只改版本檔(應該通過)。

結論

方向正確、實作大致嚴謹,主要建議是把「檔案有變更」的檢查加強為「版本值真的有遞增」,這樣才能真正達成 PR 描述中「避免使用者端 IndexedDB 版本比對誤判」的目的;其餘為風格/慣例層級的小建議。


🤖 Generated with Claude Code

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.

2 participants