From 80aa2b216da5c805491cdf3f3a8158ebf78e3bb0 Mon Sep 17 00:00:00 2001 From: iomgaa Date: Mon, 17 Aug 2026 06:18:50 -0400 Subject: [PATCH] docs: tighten the issue #11 plan after Codex review The tenant_id rule was wrong in a way that would have shipped: the plan said reject when strip() is empty, but the design says reject leading and trailing whitespace outright. " t1" survives the weaker rule and then compares unequal to "t1" inside an RLS policy, so a caller who pads the value silently loses rows. Adds the test that guards a promise nothing else was guarding -- same messages and namespace with different meta must still hit the cache. Without it, folding meta into the key passes every other assertion and costs a full cache cold start plus a permanently lower hit rate, which degrades quietly instead of failing. Also pins _record's new parameter positions, splits the backfill-failure setup per backend (ownership check on PG, read-only file on SQLite, and says what SQLite cannot assert), puts the red-green gate on the integration task, and names the two wiki pages. --- .../2026-08-17-issue11-caller-dimensions.md | 34 +++++++++++++++---- .../plans/issue11-caller-dimensions.md | 8 +++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/research-wiki/plans/2026-08-17-issue11-caller-dimensions.md b/research-wiki/plans/2026-08-17-issue11-caller-dimensions.md index 4952470..4bc0c47 100644 --- a/research-wiki/plans/2026-08-17-issue11-caller-dimensions.md +++ b/research-wiki/plans/2026-08-17-issue11-caller-dimensions.md @@ -76,7 +76,7 @@ meta: str, # 已序列化: 空 dict → '{}' **行为**: 在 `validate_request_overlay` 之后新增 `validate_caller_dimensions()`,按 Phase 组织(照搬既有风格): -- Phase 1 `tenant_id`: `None` 直接放行;非 `str` 报错;`strip()` 后为空报错(空串是哨兵值的地盘);长度 > 128 报错。 +- Phase 1 `tenant_id`: `None` 直接放行;非 `str` 报错;**`tenant_id != tenant_id.strip()` 报错**(首尾空白一律拒绝,不是"strip 后为空才拒绝"——`" t1"` 与 `"t1"` 会在 RLS policy 的等值比较下变成两个不同租户,静默漏数据);`strip()` 后为空亦报错(空串是哨兵值的地盘);长度 > 128 报错。 - Phase 2 `meta` 键形态: 非 `str` 报错;不匹配 `^[a-z0-9_.]{1,64}$` 报错;以 `pg_` 开头报错(保留前缀)。 - Phase 3 `meta` 键数量: > 16 报错。 - Phase 4 `meta` 值: 类型不属 `(str, int, float, bool)` 报错(注意 `bool` 是 `int` 子类,先判 `bool` 无妨,两者都合法);`float` 且 `not math.isfinite(v)` 报错;`str` 且长度 > 256 报错。 @@ -84,13 +84,17 @@ meta: str, # 已序列化: 空 dict → '{}' `ChatRequest` 增两字段(见"关键接口")。字段 docstring 说明: 只读快照,库内中间件永不修改;`meta` 不进缓存 key(`cache_namespace` 已负责租户隔离,ARCH §7.5)。 +**`ChatRequest.meta` 必须保存校验函数返回的那个浅拷贝**,不是调用方传入的原 dict——否则调用方复用同一 dict 逐次改值会让已在洋葱中流转的请求跟着变(同 `overlay` 拷贝语义的理由)。 + **验收标准**: 每条红线抛 `ValueError` 且消息含 `origin`;合法输入返回浅拷贝且与入参不是同一对象。 **测试要求**(先失败后通过): -逐条红线各一个用例——`tenant_id` 空串/纯空白/超长/非 str;`meta` 键非 str/含大写/含连字符/超 64 字符/`pg_` 前缀/17 个键;值为 `list`/`dict`/`None`/`nan`/`inf`/`-inf`/超 256 字符的 str。另加合法路径用例: `tenant_id=None` + `meta={}` 放行、`meta` 值为 `bool`/`int`/`float` 有限值放行、返回值是拷贝(改返回值不影响入参)。 +逐条红线各一个用例——`tenant_id` 空串/纯空白/**`" t1"`/`"t1 "`(首尾空白)**/超长/非 str;`meta` 键非 str/含大写/含连字符/超 64 字符/`pg_` 前缀/17 个键;值为 `list`/`dict`/`None`/`nan`/`inf`/`-inf`/超 256 字符的 str。另加合法路径用例: `tenant_id=None` + `meta={}` 放行、`meta` 值为 `bool`/`int`/`float` 有限值放行、返回值是拷贝(改返回值不影响入参)。 **验证命令**: `conda run -n PolyGateway pytest tests/unit/test_types.py -v` → 全 PASS +**另需一条缓存隔离测试**(放 `tests/unit/test_cache.py` 或 `test_client.py`): 相同 `messages` + 相同 `cache_namespace`、**仅 `meta` 不同**的两次调用,第二次**仍应命中缓存**。设计明确 `meta` 不进缓存 key(`cache_namespace` 已负责租户隔离);没有这条测试,实现者顺手把 `meta` 并进 key 不会被任何断言拦住,后果是存量缓存全量冷启动且此后命中率持续偏低——这类退化不报错、只表现为变慢。 + - [ ] 提交点: `feat: validate the dimensions a caller may attach to a call` --- @@ -131,7 +135,18 @@ meta: str, # 已序列化: 空 dict → '{}' **行为**: -`_record` 增两个形参,并在传给 recorder 前归一化: `tenant_id or ''`;`json.dumps(dict(meta), sort_keys=True, ensure_ascii=False, allow_nan=False)`,空 dict 直接用字面量 `'{}'`。 +`_record` 增两个形参,**位置排在现有末参 `reasoning_tokens` 之后**(它是 keyword-only,顺序不影响调用,但与 `_COLUMNS`/端口的追加位置保持一致便于逐行比对): + +```python +async def _record( + self, *, ..., + reasoning_tokens: int | None, + tenant_id: str | None, # 新增: 未归一化,None 合法 + meta: Mapping[str, Any], # 新增: 未序列化,空 dict 合法 +) -> None: +``` + +在传给 recorder 前归一化: `tenant_id or ''`;`json.dumps(dict(meta), sort_keys=True, ensure_ascii=False, allow_nan=False)`,空 dict 直接用字面量 `'{}'`。 `allow_nan=False` 是第二道闸(主防线是 Task 1 的入口校验)——`json.dumps` 默认把 `nan` 写成 `NaN` 字面量,那不是合法 JSON,PG 的 JSONB 会拒收,失败会被 emitter 的降级 try 吞成 warning,即把调用方的输入错误变成静默丢遥测。 @@ -207,11 +222,13 @@ meta: str, # 已序列化: 空 dict → '{}' 1. **新建库**: 表列齐全,写入后读回维度一致。 2. **旧表补列(不可逆性的机械化验收)**: 手工建一张 **22 列的旧表**并插入一行,再用当前 recorder 打开它 → 补列成功、新行写入成功、**老行的 `tenant_id` 读出为空串而非 NULL**。这条直接对应 issue 的核心论点(先启用后加列,老行归属无法还原);断言"是空串"而非"是 NULL",因为 NULL 在 RLS policy 下是对所有人永久不可见的黑洞。 -3. **补列失败的降级方向**: 权限不足时逐行降级丢弃而非判死(沿用 issue #9 既有测试形态,不新造机制)。 +3. **补列失败的降级方向**: 补列失败时逐行降级丢弃而非判死(沿用 issue #9 既有测试形态,不新造机制)。**两端的失败构造方式不同,不可笼统写"各测一遍"**: + - **Postgres**: 用只有 `SELECT, INSERT ON llm_calls` 权限的角色连接——`ALTER TABLE` 的 ownership 检查早于 `IF NOT EXISTS` 的存在性判断,故必然失败。断言: 记 warning、`_failed` **未**置位、后续 INSERT 仍尝试。 + - **SQLite**: 无角色权限模型,等价构造是**文件只读**(`chmod 444` 或以 `file:...?mode=ro` 打开)。但只读库连 INSERT 也做不了,故此处只断言"补列失败不清空 `self._conn`、不抛出 `__init__`"(即 `sqlite.py:112` 那条既有纪律),**不断言"写入仍成功"**——那在只读库上本就不可能。 **验收标准**: PG 与 SQLite 行为对称;补列走既有 `_BACKFILL`,不新增 DDL 路径。 -**测试要求**: 上述三项即测试本体。Postgres 用真实实例(CLAUDE.md §4.6: Redis/PG 相关测试不 mock)。 +**测试要求**(先失败后通过): 上述三项即测试本体,**同样适用红绿证据门**——先写出断言看它因缺列/缺维度而失败,再实现至通过,保留失败输出。Postgres 用真实实例(CLAUDE.md §4.6: Redis/PG 相关测试不 mock)。 **验证命令**: `conda run -n PolyGateway pytest tests/integration/test_postgres_telemetry.py -v` → PASS @@ -223,13 +240,16 @@ meta: str, # 已序列化: 空 dict → '{}' ## Task 8: 文档同步 -**文件**: `README.md`、`CHANGELOG.md`、Gitea wiki(按 `research-wiki/docs-convention.md` §2 清单) +**文件**: `README.md`、`CHANGELOG.md`、Gitea wiki 的 `指南-遥测与成本` 与 `参考-公共API` 两页 + +按 `docs-convention.md` §2「新公共 API / 新能力」行,须同步「对应指南页 + `参考-公共API` + 侧边栏 + CHANGELOG」。本次**扩写** `指南-遥测与成本`(新增"多租户与自定义维度"一节)而非新建页,故**侧边栏与 `Home.md` 不动**——新增页才需要同步导航。若执行时判断内容多到该独立成页(如 `指南-多租户`),则必须一并改 `_Sidebar.md` 与 `Home.md` 分流表。 **行为**: - **CHANGELOG**: 新增"未发布"段,写清新增两列、两个新参数(三条链路)、校验规则与上限数值、**以及库不建索引/不启用 RLS 的边界**。 - **README**: 能力表补调用方维度;数字型断言若涉及遥测字段数,用 `inspect.signature` **实测**后再写(发布流程 §4.4.1 第 1 步的教训)。 -- **wiki**: 新增一节"多租户与自定义维度",含设计 §4.5 定稿的 RLS 模板(`ENABLE` + `FORCE` + `USING`/`WITH CHECK` 双写 + `NULLIF(current_setting(..., true), '')`)与复合索引 `(tenant_id, created_at)`,并写明三个陷阱: 表属主默认豁免 RLS;租户上下文必须在**显式事务内**用 `set_config(..., true)`(asyncpg 默认 autocommit,单发 `SET LOCAL` 会当场失效而 PG 只发 warning 不报错,表现为 fail-closed 到零行);只写 `USING` 不写 `WITH CHECK` 时租户 A 能插入标着 B 的行。 +- **`参考-公共API`**: 更新 `chat()`、`embed()`(以及 Task 6 若执行则含 OCR 两方法)的签名——该页纪律是"以源码实测为准",改前先对照实际签名,不凭计划文本写。 +- **`指南-遥测与成本`**: 新增一节"多租户与自定义维度",含设计 §4.5 定稿的 RLS 模板(`ENABLE` + `FORCE` + `USING`/`WITH CHECK` 双写 + `NULLIF(current_setting(..., true), '')`)与复合索引 `(tenant_id, created_at)`,并写明三个陷阱: 表属主默认豁免 RLS;租户上下文必须在**显式事务内**用 `set_config(..., true)`(asyncpg 默认 autocommit,单发 `SET LOCAL` 会当场失效而 PG 只发 warning 不报错,表现为 fail-closed 到零行);只写 `USING` 不写 `WITH CHECK` 时租户 A 能插入标着 B 的行。 - wiki 必须明确: **执行这些 DDL 是下游 DBA 的职责,库不会代劳**;不执行则 `tenant_id` 只是一个普通列,没有数据库层强制。 **验收标准**: 三处文档对"库做什么、下游做什么"的表述一致,不出现"库自动启用 RLS"之类的措辞。 diff --git a/research-wiki/plans/issue11-caller-dimensions.md b/research-wiki/plans/issue11-caller-dimensions.md index df32d6b..2e0c9a0 100644 --- a/research-wiki/plans/issue11-caller-dimensions.md +++ b/research-wiki/plans/issue11-caller-dimensions.md @@ -7,3 +7,11 @@ date: 2026-08-17 # 调用方自定义维度实现计划(issue #11) +正文: `2026-08-17-issue11-caller-dimensions.md`(249 行,8 个任务)。实现 `design:issue11-caller-dimensions`。 + +- **任务顺序**: 端口与两个遥测后端(Task 2)先于三条调用链(Task 4/5/6)落地——后者写入的字段必须已有列可落。Task 1(校验函数 + `ChatRequest` 字段)是全部任务的前置。 +- **计划阶段的新发现**: 设计只覆盖 `chat()`/`embed()`,核实代码发现**第三条遥测链路**——`OcrClient` 经同一 `TelemetryEmitter.emit_attempt` 写遥测(`ocr.py:426`),`_emit` 在 `ocr.py:398` 现场构造 `ChatRequest`,与 embedding 同构。OCR 行与 chat 行落**同一张表**,漏掉则多租户审计链缺一块且同样不可逆。列为 Task 6 并标注**超出已批准设计的字面范围**(与 issue #10 先例一致: 那次 issue 只报告 chat 的 400,OCR 被认定为同一缺陷的其余分支而一并修);可以不做,但必须在 CHANGELOG 声明"OCR 行不带调用方维度",**不得静默跳过**。 +- **把 issue 的核心论点钉成测试**: Task 7 要求手工建 22 列旧表 → 用当前 recorder 打开 → 断言老行 `tenant_id` 读回**空串而非 NULL**。这直接验收 issue「先启用后加列则归属无法还原」的论点,且断言方向选空串是因为 NULL 在 RLS policy 下是对所有人永久不可见的黑洞,而非"未归属"。 +- **几处易实现反的地方已写进验收**: `emit_cache_hit` 必须读**本次请求**的维度而非缓存中历史响应的(构造"请求属租户 A、缓存历史属租户 B"的用例断言落 A);`embed()` 多批时**每一批**的行都要带同一份维度(只断言首行会漏掉"只有第一批带维度"的实现);非法输入必须 `ValueError` 且 recorder **零调用**(证明校验早于遥测)。 +- **不做的事**: 不把 embedding/OCR 链上已有的四个同类参数收成值对象(任务外重构);不建索引、不启用 RLS(库只交付模板,执行是下游 DBA 职责)。 +