fix(tools): add_row.py が廃止した no を使い回さないようにし、台帳の運用手順を改める - #1917
Conversation
台帳の no は主キーで、起票・許可リスト・所見の本文から参照される。 プライベートリポジトリ側で、払い出した番号を no_registry.tsv に残し (廃止した番号も消さない)、振り直しと使い回しを検査で落とすようにした。 add_row.py は台帳だけを見て「最大値 + 1」を振っていたので、末尾の行を 廃止した直後にその番号をもう一度振ってしまう。検査で落ちるので事故には ならないが、手で直す手間が出る。 add_row.py 新しい番号は、台帳と no_registry.tsv の両方の最大値の次にする。 --append のとき no_registry.tsv にも同じ番号で書き足す。 no_registry.tsv が無ければ従来どおり台帳だけで決め、注意を出す。 ヘッダが想定と違えば何も書かずに止める。 merge.py / scripts/README.md merge.py は全行を振り直すので、初回生成専用であることを明記した。 行の追加・削除・経路の改名で no をどう扱うかの手順を書いた。 tests/test_add_row.py 修正前の add_row.py では 4 件が落ちることを確かめた。 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
Reviewer's Guideadd_row.py が台帳と no_registry.tsv の双方を参照して番号を払い出し、追記時には同じ番号をレジストリへ同期することで、廃止番号の再利用を防ぐ。レジストリ不在・不正ヘッダの扱い、既存台帳での merge.py 使用禁止などを文書化し、主要な回帰ケースをテストで補強している。 Sequence diagram for safe API number allocationsequenceDiagram
participant User
participant add_row.py
participant Ledger as full.tsv
participant Registry as no_registry.tsv
User->>add_row.py: run add_row.py
add_row.py->>Ledger: read existing no values
add_row.py->>Registry: read registered no values
add_row.py->>add_row.py: next_no(full_lines, registry_lines)
add_row.py-->>User: display proposed rows
opt --append
add_row.py->>Ledger: append new rows
add_row.py->>Registry: append registry_entry rows
Registry-->>add_row.py: status = 現役
end
Flow diagram for no_registry.tsv validation and fallbackflowchart TD
Start["Run add_row.py"] --> ReadLedger["Read full.tsv"]
ReadLedger --> RegistryExists{"no_registry.tsv exists?"}
RegistryExists -->|No| LedgerOnly["Allocate from ledger maximum\nWarn that allocation is not recorded"]
RegistryExists -->|Yes| HeaderValid{"Expected header?"}
HeaderValid -->|No| Stop["Stop without writing"]
HeaderValid -->|Yes| BothMax["Allocate after the maximum\nof ledger and registry"]
LedgerOnly --> Append{"--append?"}
BothMax --> Append
Append -->|No| Display["Display only; do not write"]
Append -->|Yes| WriteLedger["Append to full.tsv"]
WriteLedger --> WriteRegistry["Append same no to no_registry.tsv\nwith status 現役"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tools/api-inventory/scripts/add_row.py" line_range="192-202" />
<code_context>
hdr = lines[0].split('\t')
- next_no = max(int(l.split('\t')[0]) for l in lines[1:]
- if l.split('\t')[0].isdigit()) + 1
+ registry = a.registry or os.path.join(os.path.dirname(os.path.abspath(full)), REGISTRY)
+ reg_lines = (open(registry, encoding='utf-8').read().rstrip('\n').split('\n')
+ if os.path.isfile(registry) else [])
+ if reg_lines and reg_lines[0].split('\t') != REGISTRY_COLUMNS:
+ sys.exit(f'{registry} のヘッダが想定と違います: {reg_lines[0]}')
+ no = next_no(lines, reg_lines)
rows = []
for k in keys:
- rows.append(build(hdr, k, E[k], root, next_no))
- next_no += 1
+ rows.append(build(hdr, k, E[k], root, no))
+ no += 1
if a.append:
</code_context>
<issue_to_address>
**issue (bug_risk):** 2つの `add_row.py --append` が同時に実行されると、どちらも同じ台帳・記録の最大値を読み、同じ `no` を生成してから両方のファイルに追記するため、同一番号が重複して払い出される。
**Triggers:** 自動化や複数の作業者が `--append` を並行実行する場合。
**Suggested fix:** 番号の読み取りから台帳・`no_registry.tsv` への追記までを同一の排他ロックまたは原子的な予約処理で保護してください。
</issue_to_address>
### Comment 2
<location path="tools/api-inventory/scripts/add_row.py" line_range="205-212" />
<code_context>
+ no += 1
if a.append:
with open(full, 'a', encoding='utf-8') as f:
for r in rows:
f.write('\t'.join(r) + '\n')
print(f'{full} に {len(rows)} 行を追記しました。')
+ if reg_lines:
+ with open(registry, 'a', encoding='utf-8') as f:
+ for r in rows:
+ f.write('\t'.join(registry_entry(hdr, r)) + '\n')
+ print(f'{registry} に同じ番号を払い出しました。')
+ else:
</code_context>
<issue_to_address>
**issue (bug_risk):** 台帳への追記が成功した後に `no_registry.tsv` の追記が失敗すると、台帳だけに新しい番号が存在する不整合状態を残して処理が異常終了する。次回以降、その番号の払い出し記録が欠落したまま運用される。
**Triggers:** `no_registry.tsv` が読み取り専用、存在しない親ディレクトリ配下、または追記中にI/Oエラーが発生した場合。
**Suggested fix:** 2ファイルへの更新を一時ファイルと原子的な置換、または失敗時の台帳ロールバックを使って一貫して扱ってください。
</issue_to_address>
### Comment 3
<location path="tools/api-inventory/scripts/add_row.py" line_range="146" />
<code_context>
+
+
+def next_no(full_lines, registry_lines):
+ """台帳と払い出し記録のどちらでもまだ使われていない、最小の番号。"""
+ return max(_nos(full_lines) + _nos(registry_lines) + [0]) + 1
+
+
</code_context>
<issue_to_address>
**nitpick:** `next_no` のdocstringは「未使用の最小の番号」を返すと説明しているが、実装は常に全番号の最大値に1を加えた値を返すため、途中の未使用番号を返さない。
**Suggested fix:** docstringを「台帳と払い出し記録の最大値の次の番号」と実装どおりに修正してください。
```suggestion
"""台帳と払い出し記録の最大値の次の番号。"""
```
</issue_to_address>Sourcery assessment
Needs a human reviewer. 2 findings to address first, and this changes the persistent allocation of primary-key-like no values in the inventory and adds entries to no_registry.tsv; if the allocation is wrong, later tickets or allowlists could refer to the wrong route. The resulting files and references are bounded and can be corrected, but reverting the code does not remove identifiers or registry entries already written.
Blocking findings: tools/api-inventory/scripts/add_row.py:202, tools/api-inventory/scripts/add_row.py:212
| with open(full, 'a', encoding='utf-8') as f: | ||
| for r in rows: | ||
| f.write('\t'.join(r) + '\n') | ||
| print(f'{full} に {len(rows)} 行を追記しました。') | ||
| if reg_lines: | ||
| with open(registry, 'a', encoding='utf-8') as f: | ||
| for r in rows: | ||
| f.write('\t'.join(registry_entry(hdr, r)) + '\n') |
There was a problem hiding this comment.
issue (bug_risk): 台帳への追記が成功した後に no_registry.tsv の追記が失敗すると、台帳だけに新しい番号が存在する不整合状態を残して処理が異常終了する。次回以降、その番号の払い出し記録が欠落したまま運用される。
Triggers: no_registry.tsv が読み取り専用、存在しない親ディレクトリ配下、または追記中にI/Oエラーが発生した場合。
Suggested fix: 2ファイルへの更新を一時ファイルと原子的な置換、または失敗時の台帳ロールバックを使って一貫して扱ってください。
API インベントリ差分（件数のみ）
ベースラインとの差分API インベントリ差分レポート
判定: ✅ PASS (FAIL 0 / WARN 0)サマリ
変化はありません。 台帳との突き合わせスナップショット ↔ インベントリ 突き合わせ
判定: ✅ 一致 (0件)
|
private 側(RCOSDP/weko-secret)は GitHub Actions の利用枠の制限を受けて ジョブが起動しないため、台帳の検査は手元の ci/local.sh を pre-push フックで 回す運用にした。Actions で回るのは public 側だけ。 - §2-3: clone 直後に git config core.hooksPath .githooks を実行する手順と、 private 側で Actions を使わない理由を書いた。 - 台帳の検査のコマンドを python3 -m pytest から ci/local.sh に改めた。 ci/local.sh はツールを同名ブランチの先頭から、解析対象を台帳の測定 リビジョンから取り出すので、手元の worktree の状態に左右されない。 - 手順 4: no は主キーで、振り直さない・使い回さないこと、行の追加・削除・ 改名での no_registry.tsv の扱いを書いた。 - 手順 10: private 側の push でフックが検査を回すこと、private 側の PR には Actions のチェックが付かないことを書いた。 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mhaya
changed the title
🔍 Claude レビュー統合他レビューの指摘 3 件 → ✅ 妥当 0 ／ ❌ 誤検知 0 ／ 🔎 要文脈 0 ／ ☑️ 対応済み 3
1. ☑️ 対応済み full.tsv 追記成功後に no_registry.tsv 追記が失敗すると不整合が残る
指摘は full.tsv→no_registry.tsv の順で書いていた旧実装を前提にしている。現在は no_registry.tsv→full.tsv の順に入れ替わっているため、registry 書き込み失敗時は full.tsv は未変更のまま残り不整合は生じない。full.tsv 書き込み失敗時も next_no() が両ファイルの最大値を見るため番号は使い回されず、1つ欠番になるだけで済む。 根拠確認: add_row.py:213-222(書き込み順序)と test_add_row.py の test_払い出し記録に書けなければ台帳を触らない(pytest実行して pass 確認) 2. ☑️ 対応済み 並行して --append を実行すると同一 no が重複して払い出される
台帳ファイルに fcntl.flock(LOCK_EX) を取得してから番号の読み取り〜両ファイルへの追記までを直列化するようになっている。 根拠確認: add_row.py:191-223 を確認。tests/test_add_row.py の test_並行して追記しても同じ番号を二度払い出さない(8並列サブプロセス)を実行し pass を確認 3. ☑️ 対応済み next_no のdocstringが実装(常に最大値+1)と食い違っている
docstring は現在「台帳と払い出し記録の最大値の次の番号(欠番は埋めない)。」となっており、提案どおり実装と一致する説明になっている。 根拠確認: add_row.py:146-148 を確認 次にすること: 外部データの指摘3件はいずれも成立していたが、直前のコミット群(609fa8f, 7ea3eb8, 75af0d8)で書き込み順序の入れ替え、fcntl.flock による排他制御、docstring修正、および --full 不在時に空台帳を作らないようにする修正がすべて反映済み。tools/api-inventory 配下のテストを全件(243件)実行しパスを確認しており、追加で報告すべき問題は見つからなかった。 モデル sonnet ／ 2 回実行して和集合 ／ コスト $0.8708。同じ入力でも結果が揺れるため複数回まわし、一部のパスでしか挙がらなかったものには回数を添えています 他レビューを踏まえた自動レビューです。誤りが含まれることがあります。 |
API インベントリ差分（件数のみ）
ベースラインとの差分API インベントリ差分レポート
判定: ✅ PASS (FAIL 0 / WARN 0)サマリ
変化はありません。 台帳との突き合わせスナップショット ↔ インベントリ 突き合わせ
判定: ✅ 一致 (0件)
|
PR #1917 のレビュー指摘(3件)への対応。 - 払い出し記録を台帳より先に書く。台帳を書いた後に記録の書き込みが 失敗すると、台帳にだけある記録漏れの番号が残っていた。先に記録を 書けば、後で台帳が失敗しても欠番が1つできるだけで済む。 - 番号を読んでから両ファイルに書き終えるまでを flock で排他にする。 並行して --append を回すと同じ番号を二度払い出せた。ロックは台帳 そのものに掛ける(ロック用のファイルを作ると台帳の隣に紛れて commit される)。 - next_no の docstring を実装どおり「最大値の次」に直した。 テストを2件足した。並行実行のテストは、ロックを外すと3回とも落ちる ことを確かめた。 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
API インベントリ差分（件数のみ）
ベースラインとの差分API インベントリ差分レポート
判定: ✅ PASS (FAIL 0 / WARN 0)サマリ
変化はありません。 台帳との突き合わせスナップショット ↔ インベントリ 突き合わせ
判定: ✅ 一致 (0件)
|
PR #1917 のレビュー追加指摘への対応。ロックを取るために台帳を 'a' で 開いていたので、--full を打ち間違えると空の台帳ファイルを黙って作り、 意味のない行を書き込んでいた。'r+' で開き、無ければその場で止める。 テストを1件足した。'a' に戻すと落ちることを確かめた。 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
API インベントリ差分（件数のみ）
ベースラインとの差分API インベントリ差分レポート
判定: ✅ PASS (FAIL 0 / WARN 0)サマリ
変化はありません。 台帳との突き合わせスナップショット ↔ インベントリ 突き合わせ
判定: ✅ 一致 (0件)
|
1 participant
背景
API 台帳の
noは主キーで、起票・許可リスト・所見の本文から参照されている。振り直すと、それらが黙って別の経路を指す。
プライベートリポジトリ側では、払い出した番号を
no_registry.tsvに残すようにした(廃止した番号も消さない)。あわせて、振り直しと使い回しを検査で落とすようにした。
ところが
add_row.pyは台帳だけを見て「最大値 + 1」を振っていた。このため、末尾の行を廃止した直後に、その番号をもう一度振ってしまう。検査で落ちるので
事故にはならないが、手で直す手間が出る。
また、プライベートリポジトリは GitHub Actions の利用枠の制限を受けてジョブが起動しない。
そのため、台帳の検査は手元の
ci/local.shを pre-push フックで回す運用に切り替えた。Actions で回るのは public 側だけになる。
変更
add_row.pyno_registry.tsvの両方の最大値の次にする。--appendのとき、no_registry.tsvにも同じ番号で書き足す(status=現役)。no_registry.tsvが無ければ従来どおり台帳だけで決め、注意を出す。no_registry.tsvのヘッダが想定と違えば、何も書かずに止める。--registryで記録の場所を指定できる(既定は台帳と同じディレクトリ)。ツールの手順書
merge.pyは全行を振り直すので、初回生成専用であることを明記した(docstring と
scripts/README.mdの Phase 4)。scripts/README.mdのケース2に、noの扱いと「台帳から行を消す」手順を足した。行を消しても番号は詰めず、
no_registry.tsvで廃止にする。経路の表記が変わっただけなら、同じ番号のまま書き換える。
docs/OPERATIONS.mdgit config core.hooksPath .githooksを実行する手順を足した。private 側で Actions を使わない理由も書いた。
python3 -m pytestからci/local.shに改めた。noの扱い(行の追加・削除・改名)を書いた。Actions のチェックが付かないことを書いた。
確認
tests/test_add_row.pyを追加した(5件)。修正前のadd_row.pyでは 4 件が落ちる(残る 1 件は「表示だけなら記録を触らない」)。
tools/api-inventoryの単体テストはすべて通る。--appendなし)で回し、次の番号が 1054 になること、台帳を書き換えないことを確かめた。
🤖 Generated with Claude Code
Summary by Sourcery
Preserve API ledger number identity by integrating allocation history into row creation and updating the associated operating procedures.
New Features:
add_row.pyfrom reusing retired inventory numbers by allocating from the combined maximum of the ledger andno_registry.tsv.no_registry.tsvduring append operations, with configurable registry paths and validation.Bug Fixes:
Enhancements:
merge.pyas intended only for initial ledger generation because it renumbers all entries.CI:
ci/local.sh.Documentation:
Tests: