Skip to content

fix: frame JSONL index reads on LF, not str.splitlines() - #5117

Open
kokokoXUY wants to merge 1 commit into
loopx-project:mainfrom
kokokoXUY:codex/jsonl-index-lf-framing
Open

kokokoXUY wants to merge 1 commit into
loopx-project:mainfrom
kokokoXUY:codex/jsonl-index-lf-framing

Conversation

@kokokoXUY

Copy link
Copy Markdown
Contributor

Goal And Delivered Outcome

  • Goal/source and gap: four readers of the per-Goal run index frame it with str.splitlines() — history.py::repair_index_duplicates, control_plane/runtime/run_index_rebuild.py::read_index_rows, capabilities/periodic_report/pending_intent.py::_actual_work_window and control_plane/quota/monitor_poll.py::_find_monitor_poll_turn. The index is written with json.dumps(..., ensure_ascii=False), which keeps U+0085/U+2028/U+2029 inside a value verbatim, and str.splitlines() treats all three as line breaks.
  • Observable before → after: one record whose text carries U+0085 arrives as two fragments, both fail json.loads, and the row is silently dropped. Measured directly: before the change a single such record yields 0 parsed rows; after it yields the record intact.
  • Issue/task and intended base: Related to fix(capabilities): frame git machine output on LF, not str.splitlines() #5100 (whose post-merge audit named the remaining framing scope). Intended base main.

Scope And Continuation

  • Completed scope and remaining work: all four call sites frame on LF. A trailing \r from a CRLF file stays harmless because JSON treats it as whitespace, so nothing else changes.
  • Slice boundary / successor: complete within this scope. Other splitlines() uses over JSONL-shaped files were not part of this slice; the four here are the ones that parse each line as a JSON document.

Validation

  • Tested revision: a8a75278f5e9e83b63806b21b9ca64bb939acc07
  • Run state: finished
  • Input classes: synthetic
Check kind Result Public-safe evidence / limitation
unit passed New tests/control_plane/test_run_index_jsonl_framing.py, 2 passed. Failing-before/passing-after: reverting only read_index_rows to splitlines() gives 1 failed, 1 passed (the U+0085 record test fails, the ordinary-records test still passes); with the fix both pass.
static passed ruff check on the four changed modules and the new test reports All checks passed!; all four modules import cleanly after the edit.
  • Coverage and gaps: the reproduction is direct — json.dumps(..., ensure_ascii=False) on a value containing U+0085, written to a real file and read back through the real function. The other three call sites share the same defect and the same one-line shape; they are not covered by their own cases because each sits behind a larger entry point (a lock-protected repair pass, a report window and a poll lookup), and I preferred one honest end-to-end case over three shallow ones. Remote CI not run.

Frontend / Visual Evidence

Not applicable: index reading only.

@kokokoXUY
kokokoXUY force-pushed the codex/jsonl-index-lf-framing branch from a8a7527 to c83a1f2 Compare September 26, 2026 15:40
kokokoXUY added a commit to kokokoXUY/loopx that referenced this pull request Sep 26, 2026
loopx-project#5117 fixed four readers of the per-Goal run index. The same framing defect
exists in nine more places that parse one JSON document per line after
`read_text(...).splitlines()`:

- `chat_store.py` (stored chat rows)
- `doctor.py` (installation index)
- `domain_state.py` (domain state rows)
- `event_sourced_state.py` (the append-only event log)
- `domain_packs/issue_fix.py` (two readers)
- `capabilities/explore/result_log.py` (three readers)

All of these files also write with `json.dumps(..., ensure_ascii=False)`, which
leaves U+0085/U+2028/U+2029 in a value verbatim, and `str.splitlines()` treats
them as line breaks: one record becomes two fragments, `json.loads` fails on
both, and the row is dropped or reported as invalid. Frame on LF instead.

Validation: a new case in `tests/test_event_sourced_state_store.py` writes one
event whose title carries U+0085 and asserts it round-trips; it raises
`StateEventError` before the change and passes after. The other eight sites are
the same one-line shape. `pytest -q tests/test_event_sourced_state_store.py
tests/test_chat_store_input_validation.py` -> 23 passed, 1 failed, where that
failure is `test_failure_before_replace_leaves_old_stream_intact` injecting
`OSError("injected pre-publication failure")` inside the file lock; it fails
identically with these edits reverted, so it is pre-existing on this host.
`ruff check` on the changed files reports `All checks passed!`.

Signed-off-by: kokokoXUY <13682395396@163.com>
huangruiteng pushed a commit that referenced this pull request Sep 26, 2026
#5117 and #5118 corrected thirteen readers that parse one JSON document per
line. Sixteen more call sites still framed those records with
`str.splitlines()`: `benchmark_toolkit/experiment_board.py`,
`benchmark_toolkit/study_projection.py`, `issue_fix/cli_input.py` (two),
`issue_fix/discovered_issue_promotion.py`, `issue_fix/outcome_projection.py`,
`issue_fix/pr_monitor_materialization.py`,
`repository_change_window/ledger.py`, `cli_commands/lark_inbox.py`,
`coordination/runtime_shadow.py` (two), `quota/codex_session_usage.py`,
`quota/slot_accounting.py`, `runtime/stride_observation.py`,
`testing/replan_vision_closeout_behavior.py` and `planner_worker/traex.py`.

A record whose value carries U+0085/U+2028/U+2029 arrived as two fragments and
`json.loads` failed on both, so the row was dropped, reported as invalid
(`invalid local upload JSONL row`) or raised (`traex returned invalid JSONL`).
All sixteen now frame on LF, each as a one-line change that leaves the loop,
its line numbering and its error text unchanged.

Validation: `ruff check` on all fourteen changed modules reports
`All checks passed!` and all of them import cleanly. `pytest -q tests/capabilities
-k "issue_fix or ledger or traex"` -> 133 passed, 18 failed; the failures are
pre-existing host limitations (for example `git` refusing to create
`tracked.txt?` on Windows), not framing results. The end-to-end framing case
added in #5118 covers this identical one-line pattern through a real store
round-trip.

Signed-off-by: kokokoXUY <13682395396@163.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REQUEST_CHANGES:[P2] LF 分帧正确修复了 Unicode 记录丢失,但把终止 LF 产生的空项交给既有重写器,新增了尾部空行。审查完整 base-to-head:59804c78222a5f0e4e0c158965de28c03fa23757 → c83a1f23b610f2b49b30d7125ee36337802913e4。

动机

JSONL 的每个 JSON 文档由 LF 分隔,字符串内的 NEL/LS/PS 不是记录边界。写端使用 ensure_ascii=False,所以旧 reader 会丢掉完全合法的记录。修复对历史恢复、monitor Turn 查找和报告时间窗口都有真实价值;但验收应同时保持普通记录的读写形状,而不是只证明带 NEL 的单次读取恢复。本次 Unicode 读取目标已经验证,完整读写兼容目标尚未满足。

改动思路

四个原有 reader 改为按 LF 分帧,比新增格式版本或重做索引 owner 合理;没有新增状态、provider 或调度规则。需要区分“供 json.loads 消费的分片”与“供重写器逐条加 LF 的逻辑行列表”:两个只读 consumer 会忽略末尾空项,而 repair 和 collision rebuild 会把这个空项重新写出。正确的最小边界应保留字符串中的 Unicode,同时消除分隔符制造的终止 sentinel;不能恢复 splitlines,也不能笼统过滤真实空行或改变其他行号。

具体改动

完整差异是四个生产文件 12 增/4 删,以及 40 行新测试。新测试只覆盖 read_index_rows 的 NEL 和普通 parsed rows,没有约束返回的 raw_lines 被重写后的持久文件,也未独立覆盖其余三条生产路径。

关键代码讲解

  • repair_index_duplicates(loopx/history.py:706):在现有 execute 锁内按 LF 分片、按既有 identity 分组并仅去掉可修复重复。问题是 "row\n".split("\n") 的最后一项为空,而 751–759 行的重写循环对每一项再追加 LF,第一次真实修复就会多写空行。
  • read_index_rows(loopx/control_plane/runtime/run_index_rebuild.py:40):parsed rows 正确保住 NEL/LS/PS;但 raw_lines 仍含终止 sentinel,后续 apply_reviewed_collision_rebuild 的 backup 和 replacement writer(251、263 行)都重新逐项加 LF,同一缺陷也进入 collision rebuild 的持久文件边界。
  • _actual_work_window(loopx/capabilities/periodic_report/pending_intent.py:737):真实 agent/time 过滤不变。独立文件探针显示旧版把有 Unicode 文本的历史起点丢掉,head 正确保留 09:00 起点,而非退回完成时刻。
  • find_quota_monitor_poll_turn/_find_monitor_poll_turn(loopx/control_plane/quota/monitor_poll.py:420/378):保留反向查找和 Goal、Agent、Turn 过滤,head 可完整读回 Unicode row,另一 Agent 仍不能命中。

阻塞项与最小修复

[P2] 在 history repair 和 rebuild 的 raw-line 边界,移除且仅移除终止 LF 制造的最后一个空项,或者让重写/backup 保持原有逻辑行及终止方式。同步覆盖 repair 与 collision rebuild;不要以放宽测试的行数断言来掩盖实际文件变化。补充 LF/CRLF、有/无终止 LF、已有内部空行及第二次 repair 的读回测试,并保留 NEL/LS/PS 的负回归。

对主干的风险

这不是无关红灯。相同既有命令在不可变 base 上 33 项全通过;head 加上两项新测试后为 34 通过/1 失败,失败唯一落在 test_replay_survives_supported_duplicate_index_repair:期望两条逻辑行,实际第三条为空。独立真实 history repair-index-duplicates --execute 对照也验证 ASCII、NEL、LS、PS × LF/CRLF 的八个 head case 均多出一个空行;另跑真实 collision rebuild:base 的 backup 与原文件一致、无额外空行;head 的 backup 和重写索引均新增一个空行,两个事件仍保留。preview 没有副作用,重复 repair 在无重复时不再写入。head 的其余 80 项读回、agent 隔离、时间窗口和 collision 判断通过;base 的 36 项 Unicode oracle 失败正是此次需要修复的历史缺陷。

本机额外的 pending-intent 与 monitor runtime 测试 44 项通过,history duplicate smoke 通过,仓库规定 Ruff、额外两个改动文件 Ruff、20 个配置内 mypy 源文件及 diff 检查通过。这些通过项不能覆盖已复现的持久形状回归。没有查询远端 CI,未运行全仓库/Windows 测试或真实外部报告发布;测试全部使用隔离的合成 registry/runtime,没有改动活动 Goal。

语义与 CI 对齐

继续沿用现有 JSONL writer、index identity、去重分类、锁和 monitor 权限词表,没有增加共享枚举或新强制工作义务。当前要求是恢复合法记录并保留原有普通文件形状;有意 Unicode delta 合理,新增空行不是有意合同变更。修复后至少重跑 uv run --extra test python -m pytest -q tests/control_plane/test_run_index_jsonl_framing.py tests/test_history_index_write_serialization.py tests/control_plane/test_quota_void_commit_runtime.py,并分别验证两个重写 consumer 的文件/backup。该红项属于 PR regression,不能按既有故障豁免。

我的整体评价

结论 REQUEST_CHANGES。long_horizon 的数据完整性收益已经证明,但反复发生修复时不应积累分隔符造成的空行和偏移;user_experience 也应同时保留普通历史修复的兼容读回。四个 reader 在原 owner 内做小改动是适度的,不需要新的控制面抽象或强制 TS 改写。未来重构评估:仅让两个重写 consumer 共享明确的 LF 逻辑行定义可能减少重复格式知识,是否抽取由最小修复决定;报告窗口和 monitor 的业务规则不应并入通用 reader。先补齐这个很小的读写边界,再重新审查新 head;当前不能批准。

English verdict: REQUEST_CHANGES - Head c83a1f2. Unicode record recovery works, but LF splitting introduces a terminal empty item that repair/rebuild writers serialize as an extra blank line. The immutable base passes the existing replay suite; this head newly fails its line-count invariant and eight independent real-CLI cases. Fix terminal-sentinel handling and cover both rewrite consumers.

@kokokoXUY
kokokoXUY force-pushed the codex/jsonl-index-lf-framing branch from c83a1f2 to ffbe1c5 Compare September 27, 2026 02:39
kokokoXUY added a commit to kokokoXUY/loopx that referenced this pull request Sep 27, 2026
loopx-project#5117 fixed four readers of the per-Goal run index. The same framing defect
exists in nine more places that parse one JSON document per line after
`read_text(...).splitlines()`:

- `chat_store.py` (stored chat rows)
- `doctor.py` (installation index)
- `domain_state.py` (domain state rows)
- `event_sourced_state.py` (the append-only event log)
- `domain_packs/issue_fix.py` (two readers)
- `capabilities/explore/result_log.py` (three readers)

All of these files also write with `json.dumps(..., ensure_ascii=False)`, which
leaves U+0085/U+2028/U+2029 in a value verbatim, and `str.splitlines()` treats
them as line breaks: one record becomes two fragments, `json.loads` fails on
both, and the row is dropped or reported as invalid. Frame on LF instead.

Validation: a new case in `tests/test_event_sourced_state_store.py` writes one
event whose title carries U+0085 and asserts it round-trips; it raises
`StateEventError` before the change and passes after. The other eight sites are
the same one-line shape. `pytest -q tests/test_event_sourced_state_store.py
tests/test_chat_store_input_validation.py` -> 23 passed, 1 failed, where that
failure is `test_failure_before_replace_leaves_old_stream_intact` injecting
`OSError("injected pre-publication failure")` inside the file lock; it fails
identically with these edits reverted, so it is pre-existing on this host.
`ruff check` on the changed files reports `All checks passed!`.

Signed-off-by: kokokoXUY <13682395396@163.com>
@kokokoXUY

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (420782f0). The red shards on the previous head came from the
base, not from this diff: they failed in
tests/control_plane/test_checkpoint_provider_fence.py::test_public_update_before_final_read_rejects_then_reread_succeeds[file]
with

AssertionError: {... "error": "main.<locals>.native() got an unexpected keyword argument 'timeout'" }

That is the timeout-aware runtime double that #5104 aligned, and this branch was cut before
it landed. I re-ran the same test on current main locally: the [file] arm passes, so the
failure is not reproducible on the rebased base. The [sqlite] arm still fails on my host for a
different and unrelated reason (SQLite authority runtime is not qualified (SQLite 3.50.4 ...) —
my local SQLite is older than the qualification floor), which CI's runner does not hit.

The diff itself is unchanged; only the base moved.

kokokoXUY added a commit to kokokoXUY/loopx that referenced this pull request Sep 27, 2026
loopx-project#5117 fixed four readers of the per-Goal run index. The same framing defect
exists in nine more places that parse one JSON document per line after
`read_text(...).splitlines()`:

- `chat_store.py` (stored chat rows)
- `doctor.py` (installation index)
- `domain_state.py` (domain state rows)
- `event_sourced_state.py` (the append-only event log)
- `domain_packs/issue_fix.py` (two readers)
- `capabilities/explore/result_log.py` (three readers)

All of these files also write with `json.dumps(..., ensure_ascii=False)`, which
leaves U+0085/U+2028/U+2029 in a value verbatim, and `str.splitlines()` treats
them as line breaks: one record becomes two fragments, `json.loads` fails on
both, and the row is dropped or reported as invalid. Frame on LF instead.

Validation: a new case in `tests/test_event_sourced_state_store.py` writes one
event whose title carries U+0085 and asserts it round-trips; it raises
`StateEventError` before the change and passes after. The other eight sites are
the same one-line shape. `pytest -q tests/test_event_sourced_state_store.py
tests/test_chat_store_input_validation.py` -> 23 passed, 1 failed, where that
failure is `test_failure_before_replace_leaves_old_stream_intact` injecting
`OSError("injected pre-publication failure")` inside the file lock; it fails
identically with these edits reverted, so it is pre-existing on this host.
`ruff check` on the changed files reports `All checks passed!`.

Signed-off-by: kokokoXUY <13682395396@163.com>

Keep the test imports above the module constant (E402).

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REQUEST_CHANGES — 本轮基于新 head 重新审查,上一轮的持久文件形状回归仍未修复。

审阅 head:ffbe1c54a1c1bed6234d0d5ef26db0914fa1df08
不可变基线:420782f03725bf9b7603be481f2b0525beff5807

动机

JSONL 的记录由 LF 分隔,JSON 字符串里的 NEL/LS/PS 是合法数据。写端使用 ensure_ascii=False,原来的 splitlines 会拆坏这些记录,影响历史恢复、monitor Turn 查找和报告时间窗口。这个修复有明确价值,不需要新的协议或状态 owner。不过验收包含普通文件在修复后的兼容读回,而不只是 Unicode 记录能被解析。对照上一轮评审,本轮 rebase 没有消除那个阻塞。

改动思路

四个 reader 在各自既有边界改为按 LF 分帧是合适的最小方向。报告窗口和 monitor 只消费解析结果,末尾空片段会被跳过;history repair 和 collision rebuild 还消费 raw_lines,并逐项追加 LF 重写文件。这两种 consumer 的契约不能混为一谈。正确修复应仅消除终止分隔符生成的 sentinel,保留真实内部空行、行号、既有锁、identity 分类和恢复记录,不应恢复 Unicode splitlines,也不应为了通过检查简单过滤所有空行。

具体改动

完整差异是五个文件 +52/-4:四个生产 reader 的 delimiter 替换及解释注释,另有 40 行 read_index_rows 测试。没有新 CLI、provider、持久字段或调度义务。新测试验证 NEL 记录和普通 parsed rows,但未约束 raw_lines 的后续重写、backup 或其余三个 consumer。

关键代码讲解

  • repair_index_duplicates(loopx/history.py:667,改动在 706 行):锁内按 LF 拆分,再依既有 identity 去重。输入以 LF 结束时,列表多出终止空项;751–759 行将每个保留项再加 LF,真实 repair 因而新增空行。
  • read_index_rows(loopx/control_plane/runtime/run_index_rebuild.py:40):parsed rows 正确保留 Unicode;raw_lines 却把终止空项传给 apply_reviewed_collision_rebuild 的 backup/replacement writer(250–263 行),使备份和重写索引同样多出空行。
  • _actual_work_window(loopx/capabilities/periodic_report/pending_intent.py:737):原有 agent/time/boundary 过滤保留。真实文件探针确认 head 从含 Unicode 的 09:00 历史起点取窗口,而不是丢失起点后退回完成时刻。
  • find_quota_monitor_poll_turn/_find_monitor_poll_turn(loopx/control_plane/quota/monitor_poll.py:420/378):保留逆序查找和 Goal、Agent、Turn 过滤;新 head 读回完整 Unicode row,另一 Agent 仍不能命中。

阻塞项与最小修复

[P2] history.py:706 与 run_index_rebuild.py:43 的 LF 分帧仍把终止空项交给重写器,产生非预期持久字节变化。请在逻辑行边界移除且仅移除最后的终止 sentinel,或让两个 writer 明确保留既有逻辑行契约。补充两个重写 consumer 的 LF/CRLF、有/无终止 LF、内部空行、Unicode 与再次 repair 读回测试;不要放宽既有行数断言。

对主干的风险

这不是无关红 CI:相同既有 pytest 命令在不可变 base 上 26 passed;head 加两个新测试后为 27 passed/1 failed,唯一失败仍是 test_replay_survives_supported_duplicate_index_repair,两条逻辑行变成三条,末项为空。

独立同输入 real-CLI/文件矩阵覆盖 22 个场景。head 的 12 个只读组合(ASCII/NEL/LS/PS × LF/CRLF/无终止 LF)全部恢复记录、窗口和 monitor 隔离;八个 repair 组合均多写一个尾部空行,有终止 LF 的 collision rebuild 也使 backup 和 replacement 多一空行。无终止 LF 的 collision case 保留基线形状。preview 无写入,第二次无重复 repair 不再写入。base 的 Unicode 读取失败是本次有意修复的历史缺陷,不要求保留错误输出。

仓库规定的 Ruff 命令通过,配置内 mypy 20 个源文件通过,diff check 通过。本轮使用独立工作树、CPython 3.13 和隔离合成 registry/runtime;未改活动 Goal。遵照当前 profile 未查询或等待远端 CI,也未跑全仓库、Windows 或真实外部报告发布。作者提到的其他旧 CI 故障不作为本次 request-changes 理由。

语义与 CI 对齐

沿用已有 JSONL writer、index identity、锁和恢复计划契约,无新状态分类、actor 权限或工作义务;没有 default-off 声明。Unicode delta 已披露且合理,新增空行不是有意默认行为。原来的重写文件不变量仍是当前验收。修复后请重跑:

uv run --extra test python -m pytest -q tests/control_plane/test_run_index_jsonl_framing.py tests/test_history_index_write_serialization.py tests/control_plane/test_quota_void_commit_runtime.py
uv run --extra test python -m ruff check tests loopx/canary loopx/control_plane loopx/domain_packs loopx/presentation
uv run --extra test python -m mypy

并通过实际 repair/collision rebuild 验证索引和 backup 字节。

我的整体评价

结论 REQUEST_CHANGES,阻塞仅为上述已复现的 PR regression。规模与 Unicode 数据完整性问题成比例,原 owner 内修补优于新增通用框架。long_horizon 的合法记录恢复收益已证明,但反复执行修复时不应引入额外分隔符;普通历史恢复的用户读回也应兼容。未来向简化检查:两个重写 consumer 的 LF 逻辑行定义值得统一为一个很窄的边界,是否抽取由最小修复决定;报告、monitor 的业务筛选不应被合并。需要修复的是现有很小的读写边界,不是承担无关 CI 或整项状态迁移。

English verdict: REQUEST_CHANGES - Head ffbe1c5 still adds a terminal blank line in repair and collision rebuild. The immutable base passes 26 existing tests; head newly fails the replay line-count invariant, and independent real-CLI cases confirm both rewrite defects. Unicode read recovery is valid; fix terminal-sentinel handling without weakening the invariant.

huangruiteng pushed a commit that referenced this pull request Sep 27, 2026
#5117 fixed four readers of the per-Goal run index. The same framing defect
exists in nine more places that parse one JSON document per line after
`read_text(...).splitlines()`:

- `chat_store.py` (stored chat rows)
- `doctor.py` (installation index)
- `domain_state.py` (domain state rows)
- `event_sourced_state.py` (the append-only event log)
- `domain_packs/issue_fix.py` (two readers)
- `capabilities/explore/result_log.py` (three readers)

All of these files also write with `json.dumps(..., ensure_ascii=False)`, which
leaves U+0085/U+2028/U+2029 in a value verbatim, and `str.splitlines()` treats
them as line breaks: one record becomes two fragments, `json.loads` fails on
both, and the row is dropped or reported as invalid. Frame on LF instead.

Validation: a new case in `tests/test_event_sourced_state_store.py` writes one
event whose title carries U+0085 and asserts it round-trips; it raises
`StateEventError` before the change and passes after. The other eight sites are
the same one-line shape. `pytest -q tests/test_event_sourced_state_store.py
tests/test_chat_store_input_validation.py` -> 23 passed, 1 failed, where that
failure is `test_failure_before_replace_leaves_old_stream_intact` injecting
`OSError("injected pre-publication failure")` inside the file lock; it fails
identically with these edits reverted, so it is pre-existing on this host.
`ruff check` on the changed files reports `All checks passed!`.

Signed-off-by: kokokoXUY <13682395396@163.com>

Keep the test imports above the module constant (E402).
Four readers of the per-Goal run index frame it with `str.splitlines()`
(`history.py::repair_index_duplicates`, `run_index_rebuild.py::read_index_rows`,
`pending_intent.py::_actual_work_window`,
`monitor_poll.py::_find_monitor_poll_turn`). The index is written with
`json.dumps(..., ensure_ascii=False)`, which keeps U+0085, U+2028 and U+2029
inside a value verbatim — and `str.splitlines()` treats all three as line
breaks. A record holding one in a text field arrives as two fragments, both
fail `json.loads`, and the row is silently dropped.

Frame on LF instead. A trailing `\r` from a CRLF file stays harmless because
JSON treats it as whitespace.

Reproduced before the change: one record containing U+0085 yields zero parsed
rows; after it yields the record intact. The new test fails on
`read_index_rows` when only that function is reverted to `splitlines()` and
passes with the fix.

Validation: `pytest -q tests/control_plane/test_run_index_jsonl_framing.py` ->
2 passed; `ruff check` on the five files reports `All checks passed!`.

Signed-off-by: kokokoXUY <13682395396@163.com>

Rebased onto current main.
@kokokoXUY
kokokoXUY force-pushed the codex/jsonl-index-lf-framing branch from ffbe1c5 to e05b0dc Compare September 27, 2026 05:17

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REQUEST_CHANGES — 新 head 的完整审查确认上轮尾部空行回归仍在,并发现一个更严重的 monitor-poll 调用破坏。

Reviewed head: e05b0dc
Baseline: 03b7e66

动机

JSONL writer 保留字符串中的 NEL/LS/PS;这些字符不是记录分隔符,旧 splitlines 会拆坏并丢掉合法行。修复四个已有 reader 有真实的数据完整性价值。但当前目标还包括普通历史恢复的文件兼容,以及原有 monitor 后续工作的继续执行,不能只以新读取测试通过判定完成。上轮 head ffbe1c5 的分帧/重写逻辑在相关文件中没有得到修复;本轮已重新运行不可变 base 与新 head,不继承旧结论。

改动思路

在原 reader 内改为 LF 是最小、合理的设计,不需要新状态、格式版本或通用框架。只读 reader 可以跳过终止空片段;repair 与 collision rebuild 则把 raw_lines 逐项重新追加 LF,因此必须在逻辑行边界处理终止 sentinel。另一处与分帧无关的删除撤掉了 #5159 的 auxiliary_settlement_todo 桥接参数和投影,而 quota facade 的调用仍保留它。这不是合法的有意接口迁移,也不是无关 CI 红灯。

具体改动

完整 base-to-head 为五个文件 +52/-16:四个 reader 的 LF 改动、monitor 函数参数与辅助 Todo 投影删除,以及 40 行新测试。生产路径仍是已有 history CLI、quota monitor-poll、碰撞恢复和报告窗口;没有 frontend 设置变更或新 capability。

关键代码讲解

  • record_quota_monitor_poll_for_decision(loopx/control_plane/quota/monitor_poll.py:671,删除处约 699 行):新 head 不接受 auxiliary_settlement_todo,但 loopx/quota.py:1162–1174 每次都传这个关键字,包括值为 None 的普通调用。在真实 CLI 路径中,参数绑定先抛 TypeError,native commit、观察记录和 successor 写回均未发生;外层把它包装为 quota_state_shape_error。
  • repair_index_duplicates(loopx/history.py:667;分帧 706 行):现有锁、identity 和去重分类未变,但以 LF 结束的文件多出终止空项。751–759 行的重写器又给它追加 LF,于是一次真实修复多写空行。
  • read_index_rows(loopx/control_plane/runtime/run_index_rebuild.py:40):Unicode parsed rows 恢复正确;raw_lines 的同一 sentinel 被 apply_reviewed_collision_rebuild 的 backup/replacement writer(约 250–263 行)写进持久文件。两个事件未丢失,但备份和替换文件均发生非预期变化。
  • _actual_work_window(pending_intent.py:737)及 _find_monitor_poll_turn(monitor_poll.py:378):独立文件输入确认 ASCII/NEL/LS/PS 的 LF、CRLF、无终止 LF 十二个只读组合均恢复记录、正确窗口起点和 Agent 隔离。这里的收益不消除另外两个写边界的问题。

阻塞项与最小修复

[P1] 恢复原 auxiliary_settlement_todo 参数及必要的 decision 投影,保持与当前 quota facade、TS admission 的接口一致;不要只删除 facade 调用,因为这会撤掉 #5159 对已完成主 Todo 后辅助 monitor 的合法恢复。加真实 CLI 的普通 monitor 和 auxiliary monitor 回归。

[P2] 在两个 raw-line 重写边界移除且仅移除终止 LF 产生的 sentinel,保留真实内部空行与行号。覆盖 repair 和 collision rebuild 的 LF/CRLF、有/无终止 LF、内部空行、Unicode、preview 无副作用及再次执行。不要放宽既有 replay 行数断言。

对主干的风险

本轮相同既有测试在 base 为 29 passed;head 加两项新测试后为 30 passed/1 failed,唯一失败为 replay duplicate repair 的两行变三行。独立 22 场景文件/CLI矩阵再次确认:八个 repair 组合多写空行,有终止 LF 的 collision backup 与 replacement 也多一空行;无终止 LF 的 collision 保留基线形状。preview 无写入,第二次无重复 repair 不再改文件。base 的 Unicode oracle 失败是有意修复的旧缺陷,不要求保持错误输出。

额外的 native monitor/successor suite 在 base 13 passed,head 7 passed/6 failed。失败走真实 CLI、File/隔离 SQLite,均呈 quota collection failed。边界只读记录的独立 CLI 探针明确捕获 TypeError: record_quota_monitor_poll_for_decision() got an unexpected keyword argument 'auxiliary_settlement_todo':base exit 0 并完成写回,head exit 1、无写回。这条反例证明普通 monitor 工作被中断,不能用新 reader 单测代替。

仓库规定 Ruff、配置内 mypy(20 源文件)、额外改动文件 Ruff、diff check 均通过。全部验证使用源码工作树、兼容 Python 和隔离合成状态,不接触活动项目。未查询或等待远端 CI,未跑全仓库/跨平台 fleet,未执行真实外部报告发送;这些范围限制与两个已复现的 PR regression 分开记录。

语义与 CI 对齐

保留已有 JSONL、去重与恢复协议、锁、Agent 过滤和 monitor authority;没有新默认开关、actor 权限或强制工作义务。Unicode delta 已披露且合理;辅助 Todo 参数删除与额外空行未披露且不合理。当前需要恢复的是现有 admission 与持久读回契约,不是新增 registry 词表或放宽预算。重审至少运行:

uv run --extra test python -m pytest -q tests/control_plane/test_run_index_jsonl_framing.py tests/test_history_index_write_serialization.py tests/control_plane/test_quota_void_commit_runtime.py tests/control_plane/test_quota_monitor_poll_runtime.py
uv run --extra test python -m pytest -q tests/control_plane/test_native_monitor_poll.py tests/control_plane/test_monitor_successor_validation.py

并验证实际 repair/collision 的文件与 backup 字节。

我的整体评价

REQUEST_CHANGES。长期数据恢复收益明确,但当前完整 PR 会阻断普通 monitor 的观察/后续工作,并改变修复后的普通文件形状;long_horizon 与 user_experience 都是 regression。修复方向本来成比例,当前最小动作是恢复无关删除并修补终止 sentinel,而不是另建框架。未来简化检查:两个重写 consumer 可以共享很窄的 LF 逻辑行边界;报告窗口与 monitor 的业务过滤应保留各自 owner。本轮没有修代码或合并,只发布真实验证后的阻塞。

English verdict: REQUEST_CHANGES
Head e05b0dc restores Unicode reads but still introduces terminal blank lines in repair and collision rebuild. It also removes auxiliary_settlement_todo while the facade always passes that keyword, breaking real monitor-poll before commit. Immutable-base comparisons reproduce both PR regressions. Restore the established bridge and fix terminal-sentinel handling without weakening existing invariants.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants