feat(metering): wrapper 完整版 invoke 聚合+hash 链（W2-C3 .github#216，ADR-0062） - #54
Merged
Conversation
…ithub#216，ADR-0062） - pipeline/metering/：metering-wrapper.sh（bash 入口，直连 provider API AR-3 一期； 流式 SSE 分片在 emit 单点聚合——spike T8 分片重复计数教训）、metering.py （emit/verify/aggregate/relink，零 jq 依赖）、record.schema.json（BEH-09 字段， 内置最小 schema 校验器）、metering-verify.sh（验链）、scan-direct-sdk.sh + scan-patterns.yaml（INV-06 直连模式表，版本化）、ledger-sync.sh（metering-ledger 分支数据源管道：relink 续接+写回前整链复验） - 自测 selftest/（零真实 LLM 调用，--replay-file 离线回放）：T1 聚合恰一条+invoke_id 去重执法、T2 BEH-09 字段齐全、T3 篡改负控制（改中间字段→验链红）、T4 扫描双形态、 T5 按角色档归账、T6 llm-usage/v1 兼容件、T7 ledger-sync 干跑——入 ci.yml 新 job - 接线：llm-connectivity/spec-author 调用改经 wrapper（--role probe/spec-author； 兼容件保既有下游零改动）；llm-connectivity 增账本验链+同步步（job 级 contents:write）； spec-author 账本不外推（App token 目标仓单仓作用域 AG-2，随 conductor 波次接入）
|
Warning Review limit reached
Next review available in: 12 minutes Limit details: You’ve used all 10 included reviews currently available. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (18)
Comment |
PR Summary by Qodofeat(metering): invoke 聚合 + hash 链账本 + 直连扫描 + ledger 同步
AI Description
Diagram
High-Level Assessment
Files changed (18)
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a “full” LLM metering wrapper pipeline for CI-Workflows, aiming to ensure each LLM invoke is measurable/auditable (single aggregated record per invoke), tamper-evident (hash chain), and enforceable (INV-06 direct-SDK static scan), with CI selftests and workflow wiring to produce/verify/sync ledgers.
Changes:
- Added a new metering core (
metering.py) + JSON Schema (record.schema.json) implementing invoke aggregation, schema assertions, hash chaining, verification, and role-based aggregation (incl. llm-usage/v1 compatibility output). - Added ledger operational tooling: verification wrapper, direct-SDK scanner (configurable patterns), ledger sync to
metering-ledger, plus an offline replay selftest suite wired into CI. - Updated workflows to route LLM calls through the new wrapper and (for connectivity) verify + sync the ledger.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pipeline/metering/selftest/run-selftest.sh | Adds offline replay + negative-control selftests T1–T7 for wrapper/metering pipeline. |
| pipeline/metering/selftest/fixtures/stream-response.sse | Adds SSE stream fixture used to validate chunk aggregation and usage extraction. |
| pipeline/metering/selftest/fixtures/stream-expected.txt | Adds expected aggregated stream output fixture. |
| pipeline/metering/selftest/fixtures/prompt.txt | Adds prompt fixture for offline replay tests. |
| pipeline/metering/selftest/fixtures/plain-response.json | Adds non-stream response fixture for emit/compat output tests. |
| pipeline/metering/selftest/fixtures/ok-via-wrapper.sh | Adds “compliant” fixture for direct-SDK scan selftest. |
| pipeline/metering/selftest/fixtures/bad-with-allow.py | Adds scan fixture demonstrating inline allow marker handling. |
| pipeline/metering/selftest/fixtures/bad-direct.py | Adds intentionally non-compliant fixture to ensure scanner detects violations. |
| pipeline/metering/scan-patterns.yaml | Adds versioned, configurable direct-SDK detection patterns + exemptions. |
| pipeline/metering/scan-direct-sdk.sh | Adds scanning entrypoint implementing pattern-based detection and inline exemptions. |
| pipeline/metering/record.schema.json | Adds metering-record/v1 JSON Schema for fail-closed record validation. |
| pipeline/metering/metering.py | Adds core logic: request building, response aggregation, emit/verify/aggregate/relink. |
| pipeline/metering/metering-wrapper.sh | Adds unified metering wrapper for online calls and offline replay; emits ledger records. |
| pipeline/metering/metering-verify.sh | Adds CLI wrapper for verifying ledger chain validity. |
| pipeline/metering/ledger-sync.sh | Adds contents-API based sync pipeline to a ledger branch with relink + re-verify gates. |
| .github/workflows/spec-author.yml | Switches spec-author workflow to use the new wrapper + verify + usage-compat output. |
| .github/workflows/llm-connectivity.yml | Switches connectivity probe to wrapper; adds ledger verify + sync; adds concurrency + job-level permissions. |
| .github/workflows/ci.yml | Adds metering-selftest job and wires it into the gate. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+149
to
+153
| if not stream: | ||
| try: | ||
| body = json.loads(raw) | ||
| except json.JSONDecodeError as e: | ||
| return "", 0, None, f"响应非 JSON：{e}" |
Comment on lines
+471
to
+473
| p = sub.add_parser("verify") | ||
| p.add_argument("--dir", default=None, help="账本目录（验全部周片）；与 --file 互斥省略") | ||
| p.add_argument("--file", help="单个账本片（断点定位）") |
Comment on lines
+56
to
+71
| hits, allows = 0, 0 | ||
| for root in paths: | ||
| base = root if os.path.isdir(root) else (os.path.dirname(root) or ".") | ||
| if os.path.isfile(root): | ||
| walk = [(os.path.dirname(root) or ".", [], [os.path.basename(root)])] | ||
| else: | ||
| walk = os.walk(root) | ||
| for dirpath, dirnames, filenames in walk: | ||
| dirnames[:] = [d for d in dirnames if d not in (".git", "__pycache__", "node_modules")] | ||
| for fn in filenames: | ||
| if not any(fnmatch.fnmatch(fn, g) for g in globs): | ||
| continue | ||
| full = os.path.join(dirpath, fn) | ||
| rel = os.path.relpath(full, base).replace("\\", "/") | ||
| if any(e.search(rel) for e in exempt): | ||
| continue |
Comment on lines
+8
to
+28
| "required": [ | ||
| "schema", | ||
| "invoke_id", | ||
| "role", | ||
| "model", | ||
| "ts_start", | ||
| "ts_end", | ||
| "prompt_version", | ||
| "prompt_bytes", | ||
| "sampling", | ||
| "usage", | ||
| "latency_ms", | ||
| "http_status", | ||
| "exit_status", | ||
| "stream", | ||
| "chunks", | ||
| "artifacts", | ||
| "record_index", | ||
| "prev_record_sha256", | ||
| "record_sha256" | ||
| ], |
Code Review by Qodo
1. Remote records silently skipped
|
Comment on lines
+73
to
+77
| REMOTE_N=$(wc -l <"$BASE" | tr -d ' ') | ||
| LOCAL_M=$(wc -l <"$LOCAL" | tr -d ' ') | ||
| if [[ "$REMOTE_N" -ge "$LOCAL_M" ]]; then | ||
| echo "OK $NAME：远端 $REMOTE_N 条 ≥ 本地 $LOCAL_M 条——本片无需同步（幂等跳过）" | ||
| continue |
There was a problem hiding this comment.
1. Remote records silently skipped 🐞 Bug ≡ Correctness
ledger-sync.sh incorrectly treats a remote shard with at least as many lines as the current local shard as already synchronized, without verifying invoke_id or record/content identity, so new runs are silently skipped. Because each llm-connectivity workflow run typically produces a fresh one-record local .metering shard while the remote shard is cumulative, every run after the first fails to upload and downstream cost-check ledger data is continuously missing.
Agent Prompt
## Issue description
Remote line count is not evidence that the current run’s local invoke records are already present on the remote ledger; using `REMOTE_N >= LOCAL_M` as an idempotency check causes new, different `invoke_id` records (often the only record in an ephemeral local `.metering`) to be silently skipped and never uploaded.
## Issue Context
Each workflow run generally produces a small independent local shard (commonly a single new record) while the remote shard is cumulative across runs, so length comparisons between two independent ledgers do not prove content equality. The script currently skips before `relink`, even though `relink` is what can connect the local shard onto the remote chain tail; the sync decision should instead verify whether the remote actually already contains the local record(s) (e.g., by `invoke_id` or record/hash identity and chain contents). Preserve existing concurrency PUT protections (SHA/conflict protection).
## Fix Focus Areas
- pipeline/metering/ledger-sync.sh[70-81]
- pipeline/metering/metering.py[397-428]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+281
to
+285
| prev, index = None, 0 | ||
| if os.path.isfile(shard_path): | ||
| records, _ = read_records(shard_path) | ||
| if records: | ||
| prev, index = records[-1]["record_sha256"], len(records) |
There was a problem hiding this comment.
2. Concurrent appends break chain 🐞 Bug ☼ Reliability
emit reads the shard tail, derives the next record_index and predecessor hash, and appends the JSONL record without mutual exclusion, so concurrent wrapper invocations can both use the same observed tail and index/prev values. Both calls can return success while leaving a forked/invalid JSONL hash-chain sequence that later fails verification and downstream accounting.
Agent Prompt
## Issue description
Concurrent `emit` calls can read the same shard tail, compute identical chain fields (`record_index` and predecessor hash), and append two “successful” records that permanently fork/break the per-week JSONL hash chain and later fail verification/accounting.
## Issue Context
Tail reading, cross-file `invoke_id` de-duplication, chain field computation (`index`/`prev`/`hash`), and the actual append-to-disk must be treated as a single mutually exclusive transaction per ledger directory/shard, and the locking must work across separate wrapper processes. The lock must cover more than just the final `write()` call; otherwise two processes can still derive chain fields from the same old tail and produce conflicting records.
## Fix Focus Areas
- pipeline/metering/metering.py[281-317]
- pipeline/metering/metering.py[212-238]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+282
to
+285
| if os.path.isfile(shard_path): | ||
| records, _ = read_records(shard_path) | ||
| if records: | ||
| prev, index = records[-1]["record_sha256"], len(records) |
There was a problem hiding this comment.
3. Invalid ledger accepts appends 🐞 Bug ☼ Reliability
emit ignores parse errors and never verifies the existing shard before deriving its tail, so a malformed, tampered, or already broken ledger still accepts another record and the wrapper reports success. This violates the wrapper's fail-closed contract and extends an untrustworthy audit chain until a separate optional verification is run.
Agent Prompt
## Issue description
Emit appends to an existing ledger without validating its JSON, schema, hashes, indexes, or invoke uniqueness first.
## Issue Context
Validation must occur inside the same locked append transaction so the checked state cannot change before writing.
## Fix Focus Areas
- pipeline/metering/metering.py[281-291]
- pipeline/metering/metering.py[212-238]
- pipeline/metering/metering.py[316-317]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+76
to
+78
| for lineno, line in enumerate(text.splitlines(), 1): | ||
| for pid, desc, rx in compiled: | ||
| if rx.search(line): |
There was a problem hiding this comment.
4. Multiline calls bypass scanner 🐞 Bug ≡ Correctness
The scanner applies every regex to one physical line, while the curl and HTTP-library patterns require the client name and provider hostname on that same line. A direct call split with shell continuation or a multiline Python call passes INV-06 scanning despite directly invoking a provider.
Agent Prompt
## Issue description
Line-by-line matching misses direct provider calls whose command and endpoint appear on different lines.
## Issue Context
At minimum, normalize shell continuations and scan multiline text; preferably parse supported languages or add robust multiline patterns and negative fixtures.
## Fix Focus Areas
- pipeline/metering/scan-direct-sdk.sh[72-85]
- pipeline/metering/scan-patterns.yaml[28-33]
- pipeline/metering/selftest/run-selftest.sh[142-151]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+58
to
+60
| base = root if os.path.isdir(root) else (os.path.dirname(root) or ".") | ||
| if os.path.isfile(root): | ||
| walk = [(os.path.dirname(root) or ".", [], [os.path.basename(root)])] |
There was a problem hiding this comment.
5. Explicit scans ignore exemptions 🐞 Bug ≡ Correctness
For explicitly supplied paths, exemption matching uses paths relative to the scanned directory or file parent rather than the repository root. Scanning pipeline/metering/ directly therefore sees metering-wrapper.sh, not pipeline/metering/metering-wrapper.sh, and falsely flags the wrapper's authorized provider curl.
Agent Prompt
## Issue description
Explicit-path scans compute relative paths from inconsistent bases, preventing configured repository-relative exemptions from matching.
## Issue Context
Use the configured repository root as the stable base for all files, including explicitly supplied files/directories, and add tests for both forms.
## Fix Focus Areas
- pipeline/metering/scan-direct-sdk.sh[57-70]
- pipeline/metering/scan-patterns.yaml[14-16]
- pipeline/metering/selftest/run-selftest.sh[142-151]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+49
to
+50
| --role) ROLE="${2:?}"; shift 2 ;; | ||
| --tag) TAG="${2:?}"; shift 2 ;; |
There was a problem hiding this comment.
6. Tag argument is discarded 🐞 Bug ≡ Correctness
The wrapper parses and documents --tag but never passes it to emit; compatibility records hard-code tag to the accounting role instead. The changed connectivity workflow explicitly supplies --role probe --tag connectivity, yet its llm-usage/v1 output now says probe, breaking the prior wrapper's stage-tag semantics.
Agent Prompt
## Issue description
The accepted `--tag` value is discarded, so compatibility output changes caller-visible metadata.
## Issue Context
Keep role as the ledger accounting key, but pass tag separately through emit and use it for the `llm-usage/v1` compatibility record and filename.
## Fix Focus Areas
- pipeline/metering/metering-wrapper.sh[37-61]
- pipeline/metering/metering-wrapper.sh[116-130]
- pipeline/metering/metering.py[321-334]
- .github/workflows/llm-connectivity.yml[54-56]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+14
to
+16
| exempt_paths: | ||
| - '^pipeline/metering/' | ||
| - '^scripts/llm-call\.sh$' |
There was a problem hiding this comment.
7. 扫描豁免覆盖整个目录 🐞 Bug ⛨ Security
pipeline/metering/ 被整体排除而非仅排除 wrapper 实现，因此任何后来加入该目录的 Python、Shell 或 JS 文件都可直连 provider/SDK 而 CI 扫描仍会通过。该目录级豁免直接绕过了仓级 INV-06 门禁。
Agent Prompt
## Issue description
扫描配置将整个 `pipeline/metering/` 子树排除。扫描器在应用任意 SDK/provider 正则前即跳过匹配路径，故该目录中新建的直连调用不会被发现。
## Issue Context
wrapper 自身确实需要调用 provider，但豁免应是最小路径集合。自测 fixture 也应按精确路径处理，避免扩大可绕过范围。
## Fix Focus Areas
- pipeline/metering/scan-patterns.yaml[14-16]
- pipeline/metering/scan-direct-sdk.sh[63-85]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+151
to
+153
| body = json.loads(raw) | ||
| except json.JSONDecodeError as e: | ||
| return "", 0, None, f"响应非 JSON：{e}" |
There was a problem hiding this comment.
8. 异常响应触发未处理解包 🐞 Bug ☼ Reliability
非流式 2xx 响应在返回体不是 JSON（如 HTML/畸形）时，parse_response() 在 JSON 解析失败分支返回四个值，但 cmd_emit() 始终按三个值解包，导致先抛出 ValueError/traceback 而不是走预期的 error:metering 与 invalid-sidecar 旁证写入流程。结果是该次调用仅以 exit 3 退出且未生成对应的 invalid-*.json 审计记录，违背“总是尝试记录 invoke”的预期行为。
Agent Prompt
## Issue description
非流式响应的 JSON 解析失败路径在 `parse_response()` 中返回了与其他路径不同长度的元组（四元组 vs 三元组），而 `cmd_emit()` 固定按三值解包，导致在进入既有的 `error:metering`、invalid-sidecar 与 HTTP/transport 审计记录逻辑之前就抛出 `ValueError` 并输出 traceback。
## Issue Context
解析失败应以与正常路径一致的返回形状（或一致的结果类型/契约）交给 `cmd_emit()`，让 `cmd_emit` 继续执行现有的 fail-closed 逻辑来写入预期的计量失败旁证与 `invalid-*.json`（或对应的 HTTP/transport 错误记录），从而避免异常响应绕过“总是尝试记录 invoke”的行为。
## Fix Focus Areas
- pipeline/metering/metering.py[142-177]
- pipeline/metering/metering.py[260-315]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
动机
宪法 §4A 要求一切 LLM 调用可计量、可归账、可审计。spike T8 暴露：流式响应按分片打点导致一次 invoke 被计成 N 条（成本虚增、去重失败）；cost-check 的 LLM 预算通道数据源一直是 pending（ADR-0040 决策 6）。本 PR 把计量 wrapper 从第一期形态（scripts/llm-call.sh，ADR-0048）升完整版：invoke 聚合 + 产物 hash 链 + INV-06 静态扫描 + 账本同步管道（metering-ledger 分支，cost-check LLM 预算通道数据源——配套 .github 仓 PR 接消费端）。
决策契约：ADR-0062（已合 archive main）。
变更清单
pipeline/metering/metering-wrapper.sh——bash 入口，一切 LLM 调用唯一入口（直连 provider API，AR-3 第一期）；流式/非流式、离线回放（--replay-file，自测/重放审计）；GATE_METERING_DIR指定 JSONL 账本目录pipeline/metering/metering.py——核心单点：mkreq（请求体组装）、emit（一次 invoke 恰一条记录：SSE 分片聚合在唯一实现点、schema 断言、hash 链追加、invoke_id 去重执法、llm-usage/v1 兼容件）、verify（验链）、aggregate（按角色档归账）、relink（账本续接）。零 jq/三方依赖（内置最小 JSON Schema 校验器，fail-closed）pipeline/metering/record.schema.json——metering-record/v1：BEH-09 字段齐全（model/prompt 版本/seed/采样参数 temperature·top_p·max_tokens·thinking/输入输出 token/耗时/http+exit 状态/chunks 聚合证据/产物 hash）+ 链字段（record_index/prev_record_sha256/record_sha256）pipeline/metering/metering-verify.sh——验链工具（--dir 全部周片 / --file 断点定位）pipeline/metering/scan-direct-sdk.sh+scan-patterns.yaml——INV-06 直连静态扫描（5 类模式：裸 openai/anthropic import、client 实例化、curl/requests 直打 provider 端点），模式表版本化可配置，豁免面显式（wrapper 本体 + 一期入口 + 自测样本），行内豁免留痕（# metering-allow: <id>）pipeline/metering/ledger-sync.sh——账本同步管道：本地与远端各自验链 → relink 续接 → 合并片整链复验 → contents API 写回 metering-ledger 分支（--dry-run 预演通道）pipeline/metering/selftest/——自测（零真实 LLM 调用）+ fixtures；入 ci.yml 新 jobmetering-selftest（并挂 gate needs）llm-connectivity.yml调用改经 wrapper（--role probe）+ 新增验链/同步步（job 级 contents:write，推本仓 metering-ledger）；spec-author.yml调用改经 wrapper（--role spec-author，--usage-compat-dir 保既有下游 usage-path.txt/spec-pr.py 零改动）。钉版说明：spec-author 由 conductor 钉 ciw_ref 调用，workflow 与被 checkout 代码同源同钉点——本变更仅在钉版推进到含本 PR 的 SHA 后生效，无中间态破坏（未动任何钉版指针）AC 映射（.github#216）
stream-response.sse离线回放），When wrapper 记录，Then 账本恰 1 条、chunks=5、正文="Hello world!"、usage=终块聚合值（120/45/165）；同 invoke_id 二次落账被拒（exit 3，聚合键=invoke_id 机器执法）。证据：自测 T1/T1b/T1c 全 PASS测试方法
METERING_PYTHON=python bash pipeline/metering/selftest/run-selftest.sh→ 37 行 PASS +SELFTEST PASS：全部断言绿（零真实 LLM 调用）metering-selftestjob（自测 + 仓级 INV-06 扫描）挂入 gate风险与回滚
Card: Cloudbird-Software/.github#216