docs: correct the plans against what the code actually does
The plan review caught three mistakes that would have gone red in the tests rather than in the implementation. Column counts: COLUMNS is the insert field list and excludes the database-filled created_at, so a stale table has 23 physical columns and a current one 25, not 22 and 24. Warning capture: the library logs through loguru, which never reaches caplog, so that assertion would have passed forever without seeing a single line. And the stale-table-under-least-privilege fixture is least_privilege_pre_tenant_dsn -- the other one builds a complete table and never reaches the missing-column path at all. Three more: make lint rewrites files, so verification uses make check; the recorder signature change now ships with its only call site instead of leaving a TypeError between two commits; and the backfill statements the library runs are not the ones it prints -- the library probes first to dodge the exclusive lock, while a script handed to a DBA has to carry IF NOT EXISTS or it cannot be run twice. On the cap side, all three clients build their emitter inside __init__, so a required parameter there would strand anyone constructing a client directly. The emitter stays required, the clients take a defaulted one.
This commit is contained in:
@@ -5,7 +5,7 @@
|
||||
- **依据设计**: `research-wiki/designs/2026-08-19-issue12-telemetry-retention-design.md`(已人类审批 2026-08-19)。
|
||||
- **涉及技术**: Python 3.11+、argparse、sqlite3、asyncpg、pytest、PostgreSQL 分区与 RLS。
|
||||
- **保真校验**: **本计划不涉及参考实现迁移,保真校验不适用**。
|
||||
- **前置依赖**: **issue #13 的计划须先合并**。两条分支都会改 `config.py`(新增 settings 字段)与 `client.py`(装配透传),且本计划 Task 4 的分区模板依赖 #13 的 `telemetry_schema_sql()` 与无冲突目标的写入。本分支从 #13 合并后的 main 起。
|
||||
- **前置依赖**: **issue #13 的计划须先合并,本分支必须从合并后的 main 开出**(不可两条分支并行改再靠自动合并)。两者都动 `config.py:118-137` 的字段列表、`config.py:423-451` 的 `_load_pgw` 返回键与 `client.py:396-407` 的装配,字段顺序与返回键极易冲突且冲突后是静默的。两条分支都会改 `config.py`(新增 settings 字段)与 `client.py`(装配透传),且本计划 Task 4 的分区模板依赖 #13 的 `telemetry_schema_sql()` 与无冲突目标的写入。本分支从 #13 合并后的 main 起。
|
||||
|
||||
---
|
||||
|
||||
@@ -55,6 +55,16 @@ class TelemetryEmitter:
|
||||
) -> None: ...
|
||||
```
|
||||
|
||||
三个公共 Client 的 `__init__` 各增 keyword-only `text_cap`,**带默认值 `None`**(与既有全部可选参数同款,非破坏性):
|
||||
|
||||
```python
|
||||
class GatewayClient: # client.py:130 起的构造签名
|
||||
def __init__(self, *, ..., text_cap: int | None = None) -> None: ...
|
||||
# EmbeddingClient / OcrClient 同款
|
||||
```
|
||||
|
||||
**为什么 emitter 必填而 Client 带默认**: `TelemetryEmitter` 是库内部类,唯一构造者是这三个 Client,必填能保证没有一处漏传;而三个 Client 是**公共装配路**(下游可直接构造并注入自己的 recorder),给它们加必填参数会破坏既有调用点,且默认 `None` 恰好等于全局缺省行为(不截断)。少了这一层,直接构造的下游要么撞 `TypeError`,要么永远没法启用 cap。
|
||||
|
||||
`GatewaySettings` 新字段(无默认值),排在 `telemetry_auto_migrate` 之后:
|
||||
|
||||
```python
|
||||
@@ -80,7 +90,7 @@ telemetry_text_cap: int | None
|
||||
- [ ] **文件**: `src/polygateway/middleware/telemetry.py`、`src/polygateway/client.py`、`src/polygateway/embedding.py`、`src/polygateway/ocr.py`;`tests/unit/test_telemetry.py`、`tests/unit/test_cache.py`。
|
||||
- **行为**:
|
||||
- 按上文签名实现两个截断函数;`_record` 内在 `digest_messages(...)` 之后、`json.dumps(...)` 之前调用 `_cap_messages`,并对 `response_text`、`thinking` 调用 `_cap_text`。
|
||||
- `TelemetryEmitter` 增必填 `text_cap`;库内三个构造点(`client.py:149`、`embedding.py:131`、`ocr.py:130`)同步传参;测试内十余处构造点一并补齐。
|
||||
- `TelemetryEmitter` 增必填 `text_cap`;库内三个构造点(`client.py:149`、`embedding.py:131`、`ocr.py:130`)同步传参;**三个 Client 的 `__init__` 各增带默认值的 `text_cap` 参数**(见上,否则直接构造路要么 `TypeError` 要么永远用不上 cap);测试内十余处 emitter 构造点一并补齐。
|
||||
- **`digest_messages` 一个字节都不改**(它是缓存 key 与遥测共用的函数,`middleware/cache.py:31`)。
|
||||
- **`_cap_messages` 必须产出新对象,严禁就地修改**。这是本任务最容易踩的坑: `digest_messages` 对 content 不是 list 的消息是**原样 append 同一个 dict 对象**(`cache.py:43`),即遥测拿到的 dict 与调用方传入的、以及缓存 key 计算用的是**同一份**。就地改它会同时污染调用方的 `messages`、后续重试尝试的请求体与缓存写入的 key,且全程无任何报错。多模态 part 同理(`_digest_part` 对非 image_url 的 part 也是原样返回)。
|
||||
- `embedding.py:73` 与 `ocr.py:73` 各自的 200 字符上限**保留不动**,与新 cap 是"取更严者"的关系。
|
||||
@@ -124,8 +134,9 @@ telemetry_text_cap: int | None
|
||||
- `--older-than-days 0` 的边界(删到"此刻之前")行为明确且与文档一致。
|
||||
- 参数缺失/冲突(如 backend=sqlite 却给 `--dsn`)退出码 1。
|
||||
- `--vacuum` 不带 `--apply` 时退出码 1。
|
||||
- 先失败证据: 脚本不存在时 subprocess 返回非零且 stderr 含 `No such file`。
|
||||
- **验证**: `conda run -n PolyGateway pytest tests/unit/test_retention_tool.py -v` → PASS。
|
||||
- **PG 分支必须自带证据**(集成,真实 PG,临时 schema 隔离): ① 临时 schema 内建**分区表**,脚本探测到后打印改用 DETACH/DROP PARTITION 的提示并以退出码 **3** 结束、**一行都没删**; ② 临时 schema 内建普通表灌入跨日期的行,`--apply --batch-size 2` 后仅超期行被删且分多批提交; ③ 缺 `asyncpg` 时退出码 **2**——用一个只含 `raise ImportError` 的临时 `asyncpg.py` 目录挂进 `PYTHONPATH` 跑 subprocess 来构造该场景,不要靠 monkeypatch(脚本走的是子进程)。
|
||||
- 先失败证据: 脚本不存在时 subprocess 返回非零且 stderr 含 `No such file`;PG 三例在脚本只实现 SQLite 分支时分别以"未知 backend"或退出码 1 失败。
|
||||
- **验证**: `conda run -n PolyGateway pytest tests/unit/test_retention_tool.py tests/integration/test_retention_tool_pg.py -v` → PASS(PG 三例须在有 `PGW_TELEMETRY_PG_DSN` 的环境实跑,skip 不算通过)。
|
||||
- **提交**: `feat: add a retention script downstreams can schedule`
|
||||
|
||||
## Task 4: 生产部署模板与其机械化验收
|
||||
@@ -137,9 +148,11 @@ telemetry_text_cap: int | None
|
||||
- **分区**: `PARTITION BY RANGE (created_at)`、主键 `(call_id, created_at)`、`pg_partman` retention;并写明**分区部署下幂等键实际是 `(call_id, created_at)`**,`emit_cache_hit` 复用历史 `call_id`,故缓存命中行在普通表上第二次起会被吞掉、在分区表上每次都落一行——按 `cache_hit` 统计的下游必须知道。
|
||||
- **库需要的最小权限**: catalog SELECT(探测)+ INSERT +(可选)CREATE;auto 档另需 ALTER。
|
||||
- **合规下游推荐配置**: 一段可直接照抄的组合(`PGW_TELEMETRY_TEXT_CAP` + 分区 retention + 三角色),不把三件事散着让下游自己拼。
|
||||
- **截断覆盖面的诚实声明**(设计 §5.2,不得省): cap 作用于消息的 `content` 文本与多模态 part 中 `type == "text"` 的 `text`,与 `digest_messages` 的处理面一致;调用方放进 `tool_calls.function.arguments` 等其他字段的内容**不在覆盖范围内**。漏写这条,下游会以为开了 cap 就没有全文残留,合规判断直接出错。
|
||||
- **SQLite 侧的保留期**(设计 §6,不得省): 给按天/按实验轮转库文件的建议——这是 VT / CHSAnalyzer / dissect 三家现成的形态,比对本地文件跑 DELETE + VACUUM 更省事也更安全;`tools/telemetry_retention.py` 的 SQLite 分支是给"已经攒成一个大库"的存量场景兜底,不是推荐路径。
|
||||
- 每个代码块 ≤15 行(输出规范),超长的拆成相邻多块。
|
||||
- **验收**: 模板 SQL 在真实 PG 上逐条可执行;README 里的行为描述与实测一致。
|
||||
- **测试**(集成,真实 PG,沿用 `least_privilege_dsn` 同款临时 schema + 临时角色隔离,teardown 删净,**严禁碰共享的 `public.llm_calls`**): 新增一例,把 README 的模板 SQL 逐条执行后断言:
|
||||
- **测试**(集成,真实 PG,**新建自己的 fixture**,手法照搬 `least_privilege_dsn` 的临时 schema + 临时角色 + teardown 删净,**严禁碰共享的 `public.llm_calls`**): 新增一例,把 README 的模板 SQL 逐条执行后断言:
|
||||
- `app` 角色能 INSERT、**不能** DELETE(报权限错)。
|
||||
- `report` 角色能读、不能写。
|
||||
- 未设 `app.tenant_id` 时查询为**零行**(fail-closed),设了则只看到本租户的行。
|
||||
|
||||
@@ -33,13 +33,17 @@
|
||||
`schema.py` 的模块级常量(名称固定,两个 recorder 与公共函数共用):
|
||||
|
||||
```python
|
||||
COLUMNS: tuple[str, ...] # 24 列,顺序即物理列序(call_id 起、meta 止)
|
||||
COLUMNS: tuple[str, ...] # 24 个 INSERT 字段(call_id 起、meta 止)
|
||||
SQLITE_DDL: str # CREATE TABLE IF NOT EXISTS(全量列)
|
||||
PG_DDL: str
|
||||
SQLITE_BACKFILL: tuple[tuple[str, str], ...] # (列名, "TEXT NOT NULL DEFAULT ''")
|
||||
PG_BACKFILL: tuple[tuple[str, str], ...] # (列名, 完整 ALTER 语句)
|
||||
SQLITE_BACKFILL: tuple[tuple[str, str], ...] # 库内执行: (列名, "TEXT NOT NULL DEFAULT ''")
|
||||
PG_BACKFILL: tuple[tuple[str, str], ...] # 库内执行: (列名, 不带 IF NOT EXISTS 的 ALTER)
|
||||
```
|
||||
|
||||
**`COLUMNS` 是 INSERT 字段序,不是物理列序**: 数据库自填的 `created_at` 不在其中(它有 `DEFAULT now()`/`datetime('now')`,库从不显式写它)。**物理表列 = 24 + `created_at` = 25**;issue #11 之前的旧表则是 22 + `created_at` = 23。所有列数断言必须按物理列数写,混用两套口径是本计划最容易写错的地方(现有集成测试的 `_EXPECTED_COLUMNS` 含 `created_at`,可作对照)。
|
||||
|
||||
**库内执行的补列语句与打印给下游的语句是两份,不是一份**: 库内**不用** `ADD COLUMN IF NOT EXISTS`——PG 对它即便列已存在也会先取 ACCESS EXCLUSIVE 锁,故库侧一律"先探测后 ALTER"(`postgres.py` 现有注释已记这条实测)。而 `telemetry_schema_sql` 打印给人执行的脚本**必须**带 `IF NOT EXISTS`,否则重复执行即失败,称不上"可直接粘进迁移文件";那条语句由 DBA 在自己选的时机执行,锁风险是他的职责。
|
||||
|
||||
两个语句构造函数:
|
||||
|
||||
```python
|
||||
@@ -80,9 +84,9 @@ telemetry_auto_migrate: bool
|
||||
- 两端 DDL 文本与搬迁前逐字节相同(列名、列序、类型、默认值);`COLUMNS` 24 项且顺序未变。
|
||||
- `insert_sql("sqlite", COLUMNS)` 与搬迁前的 `_INSERT` 字符串相同;PG 侧同理(**本任务不改冲突目标**,那是 Task 2)。
|
||||
- `insert_sql` 收到非 `COLUMNS` 子集的列名抛 `ValueError`;收到未知 backend 抛 `ValueError`。
|
||||
- `telemetry_schema_sql` 输出包含全部 24 个列名,且列名出现顺序与 `COLUMNS` 一致;未知 backend 抛 `ValueError`。
|
||||
- `telemetry_schema_sql` 输出包含全部 24 个列名 + `created_at`,列名出现顺序与建表 DDL 一致;PG 变体的补列语句带 `ADD COLUMN IF NOT EXISTS`(与库内执行的那份不同,见上);未知 backend 抛 `ValueError`。
|
||||
- **测试**(`tests/unit/test_telemetry.py` 新增 `TestSchemaModule`): 上述四条各一例。先失败证据: schema.py 不存在时 import 失败。
|
||||
- **验证**: `conda run -n PolyGateway pytest tests/unit/test_telemetry.py -v` → PASS;`conda run -n PolyGateway make lint` → 通过(import-linter 契约不得报新违规: schema.py 只依赖标准库)。
|
||||
- **验证**: `conda run -n PolyGateway pytest tests/unit/test_telemetry.py -v` → PASS;`make check` → 通过(**不要用 `make lint`,它带 `ruff --fix` 会改文件、掩盖问题并污染待审 diff**;import-linter 契约不得报新违规: schema.py 只依赖标准库)。
|
||||
- **提交**: `refactor: make the telemetry schema a single source of truth`
|
||||
|
||||
## Task 2: PG 写入去掉冲突目标
|
||||
@@ -95,9 +99,10 @@ telemetry_auto_migrate: bool
|
||||
- **验证**: `conda run -n PolyGateway pytest tests/integration/test_postgres_telemetry.py -v` → PASS(无 `PGW_TELEMETRY_PG_DSN` 时 skip,**skip 不算通过**,必须在有 DSN 的环境跑一次并留下输出)。
|
||||
- **提交**: `fix: drop the conflict target so partitioned tables can accept writes`
|
||||
|
||||
## Task 3: 两个 recorder 加 `auto_migrate` 与裁剪写入
|
||||
## Task 3: 两个 recorder 加 `auto_migrate` 与裁剪写入(含 settings 字段与装配透传)
|
||||
|
||||
- [ ] **文件**: `src/polygateway/telemetry/sqlite.py`、`src/polygateway/telemetry/postgres.py`;`tests/unit/test_telemetry.py`。
|
||||
- [ ] **文件**: `src/polygateway/telemetry/sqlite.py`、`src/polygateway/telemetry/postgres.py`、**`src/polygateway/config.py`**(只加 `telemetry_auto_migrate` 字段与派生)、**`src/polygateway/client.py`**(`_build_telemetry` 透传);`tests/unit/test_telemetry.py`。
|
||||
- **为什么装配透传必须并进本任务**: `_build_telemetry` 现在调用 `PostgresRecorder(dsn)` / `SQLiteRecorder(path)`,参数一旦必填,不同步改这里整条装配路当场 `TypeError`。签名变更与其唯一调用点必须落在同一次提交,否则该提交点跑不通全套件——每个提交点都必须独立可验证。env 键解析与 `.env.example` 仍留给 Task 4。
|
||||
- **行为**:
|
||||
- 两个 recorder 的 `__init__` 增 keyword-only **必填** `auto_migrate: bool`。
|
||||
- 列探测后计算 `effective = [c for c in COLUMNS if c in existing]`(保序),据此 `self._columns` 与 `self._insert = insert_sql(backend, effective)`;`record_llm_call` 按 `self._columns` 取值。
|
||||
@@ -109,9 +114,10 @@ telemetry_auto_migrate: bool
|
||||
- 建表(`CREATE TABLE`)两档都保留,manual 只管 ALTER(设计 §4.2)。
|
||||
- **验收**: 见测试。
|
||||
- **测试**(单元,真实临时 SQLite 文件,`tmp_path`):
|
||||
- manual + 手工建的 22 列旧表 → 写入成功且能读回、`PRAGMA table_info` 列数**保持 22**(证明未 ALTER)、`caplog` 中恰有一条 warning 且同时含 `tenant_id`、`meta` 与 `ALTER TABLE`。
|
||||
- auto + 同款 22 列旧表 → 列数变 24(现状回归)。
|
||||
- manual + 全新库 → 建表且 24 列齐全(建表未被停掉)。
|
||||
- manual + 手工建的旧表(22 个 INSERT 字段 + `created_at` = **23 个物理列**) → 写入成功且能读回、`PRAGMA table_info` 行数**保持 23**(证明未 ALTER)、捕获到的 warning 恰有一条且同时含 `tenant_id`、`meta` 与 `ALTER TABLE`。
|
||||
- auto + 同款旧表 → 物理列数变 **25**(24 个 INSERT 字段 + `created_at`,现状回归)。
|
||||
- manual + 全新库 → 建表且 25 个物理列齐全(建表未被停掉)。
|
||||
- **warning 捕获不能用 `caplog`**: 库用 loguru,它不经标准 logging,`caplog` 一条也抓不到(那条断言会静默永远绿)。照搬 `tests/integration/test_postgres_telemetry.py:436` 的 `captured_warnings` fixture 形态(`logger.add(messages.append, level="WARNING")` + teardown `logger.remove`),在 `tests/unit/test_telemetry.py` 内新建同款 fixture;别命名为 `warnings`,那会遮蔽标准库模块名。
|
||||
- 缺 `call_id` 的畸形表 → warning 升级措辞,不抛异常。
|
||||
- 先失败证据: 新参数不存在时 `TypeError`;裁剪未实现时 manual 旧表用例因 `no column named tenant_id` 全行丢弃而读不回。
|
||||
- **验证**: `conda run -n PolyGateway pytest tests/unit/test_telemetry.py -v` → PASS。
|
||||
@@ -119,11 +125,10 @@ telemetry_auto_migrate: bool
|
||||
|
||||
## Task 4: 配置派生与装配
|
||||
|
||||
- [ ] **文件**: `src/polygateway/config.py`、`src/polygateway/client.py`、`.env.example`;`tests/unit/test_config.py`。
|
||||
- [ ] **文件**: `src/polygateway/config.py`、`.env.example`;`tests/unit/test_config.py`。(`GatewaySettings` 字段与 `client.py` 透传已在 Task 3 落地;本任务只补 env 键解析、派生规则与模板注释。)
|
||||
- **行为**:
|
||||
- `config.py` 增 `_SCHEMA_MODES = frozenset({"auto", "manual"})`;`_load_pgw` 内: 键未设 → `auto_migrate = telemetry_backend == "sqlite"`;键已设 → 经 `_load_choice` 校验后 `== "auto"`。**派生只写在这一处**。
|
||||
- `GatewaySettings` 增 `telemetry_auto_migrate: bool`(无默认值),`telemetry_backend == "none"` 时恒 `False`。
|
||||
- `client.py` 的 `_build_telemetry` 把它透传给两个 recorder。
|
||||
- `.env.example` 在 `PGW_TELEMETRY_BACKEND` 附近加注释行,写明三态与两端缺省的不对称及理由。
|
||||
- **验收**: 未设键 → sqlite `True` / postgres `False` / none `False`;显式 `manual` 让 sqlite 也变 `False`,显式 `auto` 让 postgres 也变 `True`;非法值报 `ValueError` 且错误信息含键名。
|
||||
- **测试**(`tests/unit/test_config.py`): 上述五条各一例。先失败证据: 字段不存在时 `AttributeError`。
|
||||
@@ -142,10 +147,10 @@ telemetry_auto_migrate: bool
|
||||
## Task 6: 真实 Postgres 集成验收
|
||||
|
||||
- [ ] **文件**: `tests/integration/test_postgres_telemetry.py`。
|
||||
- **行为**: 新增 manual 档的两例,沿用既有 `legacy_schema` / `least_privilege_dsn` fixture 的隔离纪律(临时 schema + `search_path`,teardown 删净,**严禁 DROP/TRUNCATE 共享表**)。
|
||||
- **行为**: 新增 manual 档的两例,沿用既有 `legacy_schema` / `least_privilege_pre_tenant_dsn` fixture 的隔离纪律(临时 schema + `search_path`,teardown 删净,**严禁 DROP/TRUNCATE 共享表**)。
|
||||
- **验收**:
|
||||
- manual + 22 列旧表 → `information_schema.columns` 断言**没有**新增列、写入成功、缺的两列不写、其余 22 列值正确。
|
||||
- `least_privilege_dsn`(只授 `SELECT, INSERT`,不授 schema CREATE)+ manual → 不再出现 ALTER 失败的 warning,写入照常。
|
||||
- **`least_privilege_pre_tenant_dsn`**(`tests/integration/test_postgres_telemetry.py:496`——缺列旧表 + 只授 `SELECT, INSERT` 的角色)+ manual → 不再出现补列失败的 warning,写入照常且缺的两列不写。**不要用 `least_privilege_dsn`**: 它用完整 DDL 建的是列齐全的表,压根触发不到缺列路径,那条测试会假绿。
|
||||
- **测试**: 即上述两例。先失败证据: 改动前 manual 档不存在,构造 recorder 即 `TypeError`。
|
||||
- **验证**: `conda run -n PolyGateway pytest tests/integration/test_postgres_telemetry.py -v` → PASS(必须在有 `PGW_TELEMETRY_PG_DSN` 的环境实跑,skip 不算数)。
|
||||
- **提交**: `test: prove manual mode leaves a stale table untouched`
|
||||
@@ -158,8 +163,8 @@ telemetry_auto_migrate: bool
|
||||
- CHANGELOG: 破坏性三条给"请先读这一条"待遇——① PG 不再自动补列; ② 两个 recorder 新增必填参数; ③ `GatewaySettings` 新增必填字段(影响全量注入装配路)。
|
||||
- ARCHITECTURE §7.8 补一句 schema 单一事实源与冲突目标的变化;并按设计建议新增 **D15**(库对下游库只做 SELECT/INSERT + 可选 CREATE,改结构与删数据交给下游)。
|
||||
- **验收**: README 的 SQL 片段可直接复制执行;CHANGELOG 的破坏性段落在版本条目最前;wiki 三页同步(docs-convention §2 的发版清单)。
|
||||
- **测试**: 无自动化测试;人工核对 README 片段在真实 PG 上可执行(Task 6 的环境里跑一遍)。
|
||||
- **验证**: `conda run -n PolyGateway make ci` → 全绿。
|
||||
- **测试**(集成,真实 PG,临时 schema 隔离): README 叫下游执行的就是 `telemetry_schema_sql("postgres")` 的输出,故该输出本身必须有机械化验收——在空的临时 schema 里执行一遍,断言建出的表物理列集合 == `COLUMNS` ∪ `{created_at}`;**再执行一遍,不报错**(这同时验证补列语句带 `IF NOT EXISTS` 的幂等性)。人工核对不构成可重复的回归保护,后续改 README 就会失去它。
|
||||
- **验证**: `conda run -n PolyGateway pytest tests/integration/test_postgres_telemetry.py -v` → PASS;`make ci` → 全绿。
|
||||
- **提交**: `docs: document the schema mode and the expand-contract promise`
|
||||
|
||||
---
|
||||
|
||||
Reference in New Issue
Block a user