docs: close the four gaps Codex found in the issue #11 design

The embedding client does not go through the chat onion -- embed() runs
its own chain down to _emit(), which builds a ChatRequest on the spot
and so far only fills session_id and parent_call_id. Changing chat()
alone would have left every embed row with empty dimensions, which is
exactly what the issue's second request asks for.

The bigger find: the draft claimed serialization could not fail because
the entry check already restricts values to scalars. It can. A float
passes a naive type check and json.dumps writes it as the literal NaN,
which is not valid JSON and which JSONB rejects; the failure then lands
in the emitter's degrade path and turns a caller's input error into
silently dropped telemetry. Now rejected at the entry with isfinite and
again at serialization with allow_nan=False.

Also states the validation runs at both public entries, not just chat(),
and adds the RLS template the design had promised but never wrote down.
This commit is contained in:
2026-08-17 06:03:42 -04:00
parent 61122ce437
commit b671fb629a
2 changed files with 35 additions and 7 deletions
@@ -102,16 +102,23 @@ issue 只要租户维度。人类决定放大为**调用方自定义维度**的
| `meta` key | 非空;`[a-z0-9_.]` 且长度 ≤ 64 | 照搬 OTel semconv 字符集;Langfuse 限「仅字母数字」偏严 |
| `meta` key 数量 | ≤ 16 | 量级参考: Salesforce 自定义索引 25、Loki labels 15、OTel 属性 128(对库偏宽) |
| `meta` value | 仅 `str`/`int`/`float`/`bool`;嵌套需调用方自行序列化 | OTel AnyValue 的可移植子集;后端普遍只可靠支持标量 |
| `meta` value: float | **必须 `math.isfinite`**;`nan`/`inf`/`-inf` 报错 | 见下 |
| `meta` value 长度 | `str` ≤ 256 字符 | 对齐 Sentry tag 的 200、Langfuse 的 200 量级 |
| 保留前缀 | key 以 `pg_` 开头 → 报错 | LangSmith `ls_`、Traceloop `traceloop.`、Helicone `Helicone-` 同款。**库本次不写入任何 `pg_` key**,纯预留防未来撞名 |
**超限必须报错,不得静默丢弃**。Langfuse 的做法是 value 超 200 字符直接丢弃——这条**不抄**,违反 P5「严禁默认值掩盖错误」。报错点选在 `chat()` 入口而非遥测写入点,理由是 F6: 遥测层的一切失败都被降级成 warning,校验放那里等于没有校验。这与 `overlay` 保护键的现有先例同构(`client.py:223` `validate_request_overlay`)
**非有限 float 必须在入口拒绝(Codex 审查发现)**`json.dumps({'k': float('nan')})` 产出 `{"k": NaN}`——这是 Python 的扩展语法,**不是合法 JSON**,PG 的 JSONB 会拒收。若放行,一个调用方的输入错误会变成遥测写入失败,再被 F6 的降级成 warning,即**把调用方的 bug 转化为静默丢数据**,恰好违反 P5。故两处同时收口: 入口用 `math.isfinite` 校验,序列化用 `json.dumps(..., allow_nan=False)`(实测该参数会对非有限值抛 `ValueError`),后者是入口失守时的第二道闸而非主防线
### 4.3 内部流转
**超限必须报错,不得静默丢弃**。Langfuse 的做法是 value 超 200 字符直接丢弃——这条**不抄**,违反 P5「严禁默认值掩盖错误」。报错点选在**两个公共入口**(`chat()``embed()`)而非遥测写入点,理由是 F6: 遥测层的一切失败都被降级成 warning,校验放那里等于没有校验。这与 `overlay` 保护键的现有先例同构(`client.py:223``validate_request_overlay`),校验函数同样共用一份,不在两个入口各写一遍。
### 4.3 内部流转: 两条独立链路
`ChatRequest``tenant_id: str | None = None``meta: Mapping[str, Any] = field(default_factory=dict)`(F3)。二者是**只读快照**,库内中间件永不修改——与 `sampling` 字段同一纪律(issue #4 决策 A)。
`TelemetryEmitter._record` 是唯一改动点(F5),向 recorder 多传两个参数;三个 emit 入口(`emit_attempt`/`emit_cache_hit`/`emit_terminal_failure`)统一从 `request` 读取,不各自组装。
`TelemetryEmitter._record` 是唯一的 recorder 调用点(F5),向 recorder 多传两个参数;三个 emit 入口(`emit_attempt`/`emit_cache_hit`/`emit_terminal_failure`)统一从 `request` 读取,不各自组装。
**embedding 走的是另一条链,必须单独贯穿(Codex 审查发现)**`EmbeddingClient` 不经过 chat 洋葱: `embed()``_embed_batch()``_attempt()``_emit()`,而 `_emit()``embedding.py:360` **现场构造** `ChatRequest` 仅为复用同一个 Emitter,当前只填了 `session_id`/`parent_call_id`。若只改 `chat()`,结果是 chat 行有维度而 embed 行恒为空——**恰好落空 issue 第 2 条诉求**(两条路径都要能传)。故新维度须沿这四层逐层透传,并在 `_emit()` 构造 `ChatRequest` 时填入。
**不顺手重构这条链的参数列表**: 该链已在逐层传 `session_id`/`parent_call_id`,再加两个即四个同类参数,把它们收成一个值对象在美学上更优,但那会改动 embedding 现有的全部内部签名,属任务外重构(反 gold-plating)。本次只做加法;若日后参数继续增长,再单独立项。
**缓存命中行与终态失败行同样带维度**: 前者读 `request` 而非缓存中的历史响应(维度是"本次调用由谁发起",不是历史那次);后者虽无具体源,但租户归属是已知的——这两行恰恰是审计最需要的(缓存命中意味着这次没花钱但确实发生了;终态失败意味着这个租户的请求没被服务)。
@@ -146,6 +153,20 @@ issue 只要租户维度。人类决定放大为**调用方自定义维度**的
文档必须同时告知三个陷阱: 表属主默认**豁免** RLS(需 `FORCE`);租户上下文只能用 `set_config(..., true)` 且**必须在显式事务内**(asyncpg 默认 autocommit,单发 `SET LOCAL` 会当场失效而 PG **只发 warning 不报错**,表现为策略永远拿不到租户 → fail-closed 到零行);policy 必须同时写 `USING``WITH CHECK`,只写前者则租户 A 能插入标着 B 的行。
模板正文(交付物是 wiki 用户文档的一节,此处定稿口径):
```sql
ALTER TABLE llm_calls ENABLE ROW LEVEL SECURITY;
ALTER TABLE llm_calls FORCE ROW LEVEL SECURITY; -- 属主不豁免
CREATE POLICY llm_calls_tenant_isolation ON llm_calls TO polygateway_app
USING (tenant_id = NULLIF(current_setting('app.tenant_id', true), ''))
WITH CHECK (tenant_id = NULLIF(current_setting('app.tenant_id', true), ''));
CREATE INDEX CONCURRENTLY idx_llm_calls_tenant_created
ON llm_calls (tenant_id, created_at);
```
`current_setting(..., true)` 的第二参数令 GUC 未设时返回 NULL 而非抛错,外层 `NULLIF` 把空串归一为 NULL——两者合起来使**未设租户 = 零行**(fail-closed),而不是全部行。
---
## 5. 旧版行为审计
@@ -164,13 +185,15 @@ issue 只要租户维度。人类决定放大为**调用方自定义维度**的
## 6. 非功能维度
**并发与取消**: 新增字段是不可变快照,随 `ChatRequest` 在洋葱内流转,无共享可变状态,并发调用互不干扰。取消路径不变——`TelemetryMW` 捕获 `CancelledError` 时的 `emit_terminal_failure` 同样带上维度后立即重抛,遥测写入不延迟取消传播(ARCH §5.1 约定④)。校验发生在进洋葱之前的同步代码里,不涉及 await,无取消窗口。
**并发与取消**: 新增字段是不可变快照,随请求在各自链路内流转,无共享可变状态,并发调用互不干扰。取消路径不变——`TelemetryMW` 捕获 `CancelledError` 时的 `emit_terminal_failure` 同样带上维度后立即重抛,遥测写入不延迟取消传播(ARCH §5.1 约定④)。校验在两个公共入口的同步代码里完成(chat 侧在进洋葱之前,embed 侧在切批之前),不涉及 await,无取消窗口。
**降级方向**: 分两段,方向相反且都符合铁律。**校验失败 → 报错**(`ValueError`,在 `chat()` 入口,调用方可见);**遥测写失败 → 静默降级 warning**(缓存/遥测后端不可用属"静默降级"档,不是限流/熔断的"报错而非放行"档)。补列失败 → 逐行降级丢弃,不判死。
**降级方向**: 分两段,方向相反且都符合铁律。**校验失败 → 报错**(`ValueError`,在 `chat()`/`embed()` 入口,调用方可见);**遥测写失败 → 静默降级 warning**(缓存/遥测后端不可用属"静默降级"档,不是限流/熔断的"报错而非放行"档)。补列失败 → 逐行降级丢弃,不判死。
**幂等与重复**: 不变。`call_id` 仍是主键,`ON CONFLICT DO NOTHING`/`INSERT OR IGNORE` 语义不受影响。同一 `call_id` 重复写入仍被忽略,新增两列不引入新的重复语义。
**持久化与原子性**: 不变。每行单条 INSERT,两个新列与既有 22 列在同一条语句里落盘,不存在部分写入。`meta` 的 JSON 序列化在 emitter 内完成,序列化失败被 emitter 的降级 try 捕获成 warning——但这条路径实际不可达,因为值类型已在入口校验为标量子集(§4.2),这是**双层防御**而非依赖降级兜底。
**持久化与原子性**: 不变。每行单条 INSERT,两个新列与既有 22 列在同一条语句里落盘,不存在部分写入。
`meta` 的 JSON 序列化在 emitter 内完成。**初稿曾断言"序列化失败不可达",此论断已被 Codex 审查推翻并修正**: 非有限 float 能通过"值是 `float`"这类朴素类型检查,却产出 PG 拒收的 `NaN`/`Infinity` 字面量,于是失败会落到 emitter 的降级 try 里被吞成 warning——调用方的输入错误变成静默丢遥测。修正后是真正的双层收口: 入口 `math.isfinite` 拒绝(主防线,调用方可见),序列化 `allow_nan=False`(第二道闸)。这里记下推翻过程,是因为"入口校验完备 ⇒ 下游不可能失败"这个推理模式本身容易复发。
---
@@ -180,7 +203,11 @@ issue 只要租户维度。人类决定放大为**调用方自定义维度**的
**测试策略**(合并前需先失败后通过的证据):
单元层——校验规则逐条红线(key 字符集/数量上限/value 类型/长度/`pg_` 前缀拒绝),每条断言**报错而非静默丢弃**(这是 §4.2 的核心承诺,也是与 Langfuse 分道的地方);`ChatRequest` 快照不可变;三个 emit 入口都带上维度(尤其**缓存命中行与终态失败行**——这两条最容易被漏,而它们恰是审计刚需)。
单元层——校验规则逐条红线(key 字符集/数量上限/value 类型/长度/`pg_` 前缀拒绝/**非有限 float**),每条断言**报错而非静默丢弃**(这是 §4.2 的核心承诺,也是与 Langfuse 分道的地方);`ChatRequest` 快照不可变;三个 emit 入口都带上维度(尤其**缓存命中行与终态失败行**——这两条最容易被漏,而它们恰是审计刚需)。
**两条链路各测一遍**——`chat()``embed()` 都必须有"传入维度 → 遥测行带该维度"的用例。embed 侧尤其不能省: 它经 `_embed_batch`/`_attempt`/`_emit` 四层透传,任一层漏传都不会报错、只会让维度恒为空,而这正是 issue 第 2 条诉求的落点。批量切批时**每一批的行都应带同一份维度**(维度属于本次 `embed()` 调用,不随批次变化)。
非有限 float 单独一条: 断言 `chat(meta={"x": float("nan")})``ValueError` 而**不是**写入时降级成 warning——这是 §6 记录的那个被推翻论断的机械化守卫。
集成层——真实 SQLite 与真实 Postgres 各跑一遍: 新建库列齐全;**旧表(22 列)经 `_BACKFILL` 补列后能写入**,且老行 `tenant_id` 读出为哨兵空串而非 NULL(这是 §4.4 不可逆性论证的机械化验收);补列权限不足时逐行降级而非判死(沿用 issue #9 的既有测试形态)。