test(public-safety): pin what a compacted value returns over its budget - #4955
Conversation
202 call sites pass compact_text an explicit limit, but no case in tests/ or examples/ ever crossed the budget, so the over-limit branch -- what a host or dashboard actually reads back -- had no assertion. Pin the kept head, the single ellipsis, the collapse-before-budget ordering, the exact-fit boundary, coercion, the default 220, and that the credential refusal still fires inside the kept prefix. Two behaviors are disclosed rather than pinned: a limit below 2 stops bounding the value at all, and truncation runs before the redaction check, so a credential past the cut is not refused. Signed-off-by: Yue Dai <54579099+yuedai-pbc@users.noreply.github.com>
huangruiteng
left a comment
huangruiteng
left a comment
There was a problem hiding this comment.
动机
现有 compact_text 被大量公共读模型调用，但测试没有覆盖显式 limit 下的超预算分支。此 PR 只增加这一已交付能力的回归覆盖，不改生产行为；它不是对整个 public-safety 保证的最终验收。
改动思路
直接测试现有 loopx.control_plane.runtime.public_safety 的真实函数，而不是另写 example smoke 或 mock。用多个小预算检查归一化后的截断、单个省略号和精确长度；再检查恰好容纳、None、默认预算及保留前缀内的合成凭据拒绝。测试与已有 path-shape/readmodel 覆盖互补，未新增状态或第二个规则 owner。
具体改动
仅新增 tests/control_plane/test_public_safety_text_budget.py，六个测试函数（其中一个参数化为五种预算），81 行；loopx/、配置、公共文档和 API 均未变化。超预算输出、空值、普通文本及保留前缀内的拒绝各有明确断言。
关键内容讲解
- 多预算用例断言保留
limit - 1个字符后只附加一个省略号。 - 空白折叠、恰好容纳与字符替换用例保护预算计算顺序及边界。
- 公共安全用例只验证已保留前缀内的拒绝，不声称覆盖完整原始输入的所有风险。
对主干的风险
测试本身不改变公共输出，主要风险是把当前实现的预算形状固定下来；若以后有意更改默认预算或截断形式，需要同步更新明确的调用方契约。PR 正文披露的两个既有边界缺陷仍未修复，不能因这组测试通过就宣布公共安全边界已完整合格；建议由运行时 owner 另行修复并补相应负例。由于这里没有改动运行时，本结论只批准这个独立的测试增量。
我在 exact head 运行了新增及 path-shape 测试（31 passed）、五模块相邻测试（68 passed）、现有 public-safety readmodel smoke、Ruff 与 git diff --check，均通过；未轮询远端 CI。
我的整体评价
这个单文件测试增量有独立、可持续的回归价值，不是与作者同批其他 PR 重复的 walkthrough。它用现有测试 owner 覆盖真实 helper，范围与风险相称；未发现需要本 PR 阻塞的新增回归。未来相关的小型行为保持性重构目前没有必要，优先处理正文已揭示但不属于此 PR 的运行时安全缺口。批准本测试增量，不等于批准当前 public-safety 实现的全部行为。
English verdict: APPROVE - head 522d517; focused budget regression tests are distinct and pass (31 focused, 68 adjacent plus readmodel smoke and static checks). Pre-existing safety gaps remain outside this test-only approval.
2 participants
Goal And Delivered Outcome
compact_text()is the budget every public-safety projection spendsbefore handing a value to a host, a dashboard or another tool, and
loopx/passes it anexplicit
limitat 202 call sites. No check intests/orexamples/ever called itwith a limit, so the whole over-budget branch — the one that decides what a reader
actually sees — was unasserted. The owning smoke
(
examples/control_plane/public-safety-readmodel-smoke.py) pins the whitespace, empty andlocal-path refusals at the default limit, and the existing
tests/control_plane/test_public_safety_path_shapes.pypins the two patterns andvalidate_public_safe_value; neither crosses the budget boundary. Several productionseams inject a substitute
public_safe_compact_text, which means the injected behaviorwas tested while the real one was not.
the guard passed the whole suite — removing the ellipsis, keeping one character too many,
tightening the fit test from
<=to<, comparing the length before collapsingwhitespace, dropping the
Nonenormalization, moving the default budget, removing thecredential check, and dropping the
rstrip(). After, each fails at least one case(
regression_paritybelow). The two disclosures are measured, not inferred.production file changes. Base is current
main.Scope And Continuation
cut and where the ellipsis goes (one case parametrized over five budgets), that
whitespace is collapsed before the budget is spent, that a value which exactly fits is
returned unchanged, that the ellipsis replaces a character rather than adding one, that
values are coerced and
Nonestays empty, that the default budget is 220, and that thecredential refusal still fires inside the kept prefix. Deliberately not asserted,
because both are behaviors this PR would rather see decided than frozen:
limit=0returns the input unbounded —compact[: limit - 1]becomes a negativeslice, so a 10-character value with
limit=0comes back 10 characters long;limit=1returns only the ellipsis. Every call site in
loopx/passes a literal above 1 today,but four seams forward a caller-supplied
limit(capabilities/agent_turn_recall/core.py:20,capabilities/context_providers/openviking.py:136,capabilities/issue_fix/pr_lifecycle.py:175,capabilities/issue_fix/discovered_issue_promotion.py:151), so the bound is a parameterrather than a constant. A fail-closed
max(limit, 1)or a raisedValueErroris theowner's choice, and pinning either would foreclose it.
public_safety.py:137-141), so therefusal rules only see the prefix that survives the cut. Measured on current
main:public_safe_compact_text("token=abcdefghijklmn", limit=8)returns'token=a…'rather than
None. The name promises a public-safe value; the guarantee currentlycovers only the part that was kept. Checking before truncating, or widening the checked
span to the original text, are both behavior changes and neither belongs in a test PR.
disclosure should arrive as its own change with the call sites that would be affected.
Validation
522d5179eunitpassedpython -m pytest -q tests/control_plane/test_public_safety_text_budget.py→ 11 passed: 6 new functions, one of them parametrized over five budgets.unitpassedpython -m pytest -q tests/control_plane/test_public_safety_path_shapes.py tests/control_plane/test_status_rollout_event_snapshot.py tests/control_plane/test_goal_acceptance_observation.py tests/control_plane/test_monitor_replan_agent_scope.py tests/control_plane/test_public_safety_text_budget.py→ 68 passed / 0 failed — every Python test that imports this module or injects a substitute for it, pluspython3 examples/control_plane/public-safety-readmodel-smoke.py→ exit 0 (the owning smoke still passes alongside the new cases).staticpassedpython -m ruff checkandruff format --checkclean on the new file;python -m mypy(same no-argument invocation as CI) → success in 22 source files;python -m pytest -q tests/canary→ 25 passed with the reviewed module findings unchanged.regression_paritypassedcompact_text/public_safe_compact_text, one at a time, reverted before the final run: drop the ellipsis → 9 failed;[: limit - 1]→[: limit]→ 9 failed;<= limit→< limit→ 1 failed; compare length before collapsing whitespace → 2 failed;str(text or "")→str(text)→ 1 failed; default220→240→ 1 failed; remove the credential check → 1 failed; drop.rstrip()→ 1 failed. Reverting all → 11 passed. Failing-before holds by construction: no case intests/orexamples/passed alimittocompact_texton the base revision.unitnot_runpython -m pytest -q tests/was not completed locally for this revision. The diff adds one test file and changes no module a runtime path imports, so CI's suite on the merge ref is the authoritative run here; this row is left as not run rather than reported as passing.loopx/control_plane/runtime/public_safety.py; the mutation matrix is the argument thatthe new cases bind that code. Not covered: the two disclosed behaviors above, on purpose.
Frontend / Visual Evidence
Type of Change
LoopX Area
Technical Direction
of an already-shipped public-boundary contract. No roadmap id claimed.
Shared-authority RFC fixture impact
or compatibility projection changes.
Boundary Checklist
.loopx/,.codex/goals/, and liveACTIVE_GOAL_STATE.md).none.Signed-off-bytrailer (git commit -s).