Skip to content

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

Merged
huangruiteng merged 1 commit into
loopx-project:mainfrom
kokokoXUY:codex/jsonl-index-lf-framing
Sep 29, 2026
Merged

huangruiteng merged 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).
@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.

@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 — reviewed exact head b365f195ed7ff150283f1dbce960b2d5f12b60d4 against immutable main base 69adeba1d63b02a764048a30ecc4bc2b3c2cdb02.

动机

run index 是按 LF 分隔的 JSONL。json.dumps(..., ensure_ascii=False) 会把 U+0085 等 Unicode 分隔符留在字段值里,原来的 str.splitlines() 却把它们当成记录边界,导致有效 run 被静默丢弃。修复四个真实读取入口有价值,特别是长期运行时 history、quota monitor 和周期报告不能漏掉同一条事实。

改动思路

PR 将四处 splitlines() 改成 split("\n"),用 LF 而不是 Unicode 的多种换行符分帧;新测试通过真实文件验证 read_index_rows 在 U+0085 和普通记录上的读取。这个方向与 JSONL writer 一致,但 split("\n") 对正常以 LF 结尾的文件会返回一个额外空字符串。两个读后重写入口又对每个原始元素添加 \n,把这个虚拟终止项写成真实空行。

具体改动

完整 diff 是四个生产文件各改一行,加一个 40 行的 test_run_index_jsonl_framing.py。pending_intent._actual_work_window 与 monitor_poll._find_monitor_poll_turn 只跳过空行,当前未见持久副作用;run_index_rebuild.read_index_rows 返回原始行供 collision rebuild 使用;history.repair_index_duplicates 在删除重复索引行后重写整个文件。测试仅覆盖前者的解析正例,没有覆盖两个写回消费者。没有新增状态 schema、开关、权限或前端。

关键代码讲解

  • read_index_rows 现在能把 U+0085 留在 JSON value 里,这是正确的;但返回的 raw_lines 含终止 LF 产生的空 sentinel,调用方必须过滤它。
  • repair_index_duplicates 的 rewritten = [...] 保留该 sentinel,随后 "".join(line + "\n" for line in rewritten) 把它实体化;collision rebuild 的原始内容比较与写回也复用 raw_lines,存在同类漂移。
  • 新增两项 framing 测试证明读取层的目标场景和普通场景,却没有检查“读取→修复→再次读取”的合法文件字节形态。

对主干的风险

[P1] 索引修复把终止换行复制成真实空记录。 对这个精确 head 执行 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,结果 27 passed、1 failed。失败的是现有 test_replay_survives_supported_duplicate_index_repair:修复一个重复行后,本应剩两行,实际是三行,文件末尾为 \n\n。在相同 base revision 上单独跑该测试是 1 passed,且 diff 仅改变上述分帧逻辑,因此这是 PR 自身回归,不是无关红 CI。请在保留 U+0085 语义的同时丢弃仅由最终 LF 产生的 sentinel,或在重写时只序列化真实物理记录;给 duplicate repair 与 collision rebuild 增加以 LF 结尾文件的负例,证明不会增生空行。复审时重跑上述三组测试。

本轮 git diff --check 通过。未查询或等待远端 CI;这项阻断来自本地同基线对照的确定性测试,不推断其他 CI 状态。额外空行可能被宽容 reader 跳过,但会改变持久索引的字节形态并使后续修复反复漂移,因此不能仅以解析层绿灯放行。

我的整体评价

REQUEST_CHANGES。LF 分帧是正确的局部方向,四个 call site 也确实属于同一记录契约;目前写回路径没有随之保持物理行不变量,长期累计会让索引逐次增生空行。最小改动应放在共享 read_index_rows 的 raw-line 契约或两个重写入口,不需要扩大为新的框架。代码体量合适,但要把读写回环验证补齐;本评审不授予合并权限。

English verdict: REQUEST_CHANGES - exact head b365f195ed7ff150283f1dbce960b2d5f12b60d4 fixes Unicode framing but turns terminal LF into a real blank index line during repair. The focused suite is 27 passed, 1 failed; the failing replay test passes on the immutable base. Remote CI was not polled.

Records in a JSONL run index are separated by LF. Splitting on LF was needed so
that U+0085/U+2028/U+2029 inside a value stay inside that value, but splitting on
LF also turns a file that ends in LF into a trailing empty element, and the two
consumers that serialize the raw lines back wrote that element out as a real
blank row.

Reading the result again then yields one row more than was written (two rows
became three, with a trailing blank line), so a repair that should be idempotent
keeps growing the file.

Both rewrite consumers now share `split_index_lines`, which drops exactly the
single empty element produced by a terminating LF and keeps a genuinely empty
interior row. The two read-only consumers keep the local predicate they already
use to skip blank fragments.

Verified on this host:

- `tests/control_plane/test_run_index_jsonl_framing.py` (15 tests: NEL framing,
  the physical line shape for LF/CRLF with and without a terminating LF, a second
  repair that returns `removed_row_count == 0` and leaves the bytes unchanged,
  and a collision rebuild that keeps its backup and the index bytes)
- with the sentinel fix removed, 10 of those 15 fail, including both rewrite
  consumers
- the reported set (`test_run_index_jsonl_framing.py`,
  `test_history_index_write_serialization.py`,
  `test_quota_void_commit_runtime.py`) passes
- the related quota/monitor/history suites have the same failures before and
  after the change on this Windows host (pre-existing subprocess/sqlite
  environment failures)

Signed-off-by: kokokoXUY <13682395396@163.com>
@kokokoXUY
kokokoXUY force-pushed the codex/jsonl-index-lf-framing branch from b365f19 to 7123bbe Compare September 29, 2026 14:02
@kokokoXUY

Copy link
Copy Markdown
Contributor Author

The trailing-sentinel blocker from the last review is fixed on 7123bbeac79b (I rebased onto 5ab23b3f, so the branch is no longer behind).

loopx/control_plane/runtime/run_index_rebuild.py now exposes split_index_lines, which drops exactly the single empty element a terminating LF produces and keeps a genuinely empty interior row. Both rewrite consumers use it (read_index_rows and history.repair_index_duplicates). The two read-only consumers keep the predicate they already use for blank fragments, so I left them alone.

New coverage in tests/control_plane/test_run_index_jsonl_framing.py (15 tests):

  • LF/CRLF, with and without a terminating LF: after repair_index_duplicates the file ends in exactly one \n, the row count is right, and a second repair reports removed_row_count == 0 with unchanged bytes
  • an existing interior empty row is still preserved, and a value containing U+0085 stays one record
  • the collision rebuild path: the backup is byte-identical to the original and the rewritten index is two rows with no trailing blank row

Evidence: with the sentinel fix removed, 10 of the 15 tests fail, including both rewrite consumers. With it, test_run_index_jsonl_framing.py + test_history_index_write_serialization.py + test_quota_void_commit_runtime.py are 41 passed — that also reproduces your original failure (test_replay_survives_supported_duplicate_index_repair, 3 lines instead of 2) as a red case when the fix is backed out.

Honest boundary: the wider quota/monitor/history suites have the same failure sets before and after this change on this Windows host (subprocess/sqlite environment), so they could not serve as a green signal; the three files above are the signal.

@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.

精确 head:7123bbeac79bb23aad787f4798c6ceb2e09206a9。这是对上一轮精确头评审的复审:先核对其“尾部 LF 被当作空记录”的修复,再按当前 base→head 检查完整 PR。按本次评审契约未查询或等待 GitHub CI。

动机

每个 Goal 的 runs/index.jsonl 由 json.dumps(..., ensure_ascii=False) 写入。合法 JSON 字符串中的 U+0085、U+2028、U+2029 会原样留在一条物理记录中,但 str.splitlines() 会把它们当作行边界;读者于是把一条有效记录拆成两个无法解析的片段,静默丢掉。这会使历史修复、冲突重建、配额监控轮次查找和周期报告的工作窗口与实际索引不一致。当前目标是修正这四处已确认的读者,同时保持普通 LF 文件及重写后的物理行形状。

改动思路

只把 LF 当作 JSONL 记录分隔符。两个会重写索引的路径共用 run_index_rebuild.split_index_lines:它移除恰好一个终止 LF 产生的空哨兵,保留真正的内部空行及行号;只读的监控和报告路径直接按 LF 分割,并沿用原有的空行跳过及 JSON 解析逻辑。没有新增状态、权限、CLI 或跨 Goal 决策源。该小型共享 helper 属于现有索引重建 owner;进一步抽象所有相似读者目前没有必要。

具体改动

完整 diff 为 5 个文件(4 个生产模块和 1 个 190 行定向测试,+214/−4)。read_index_rows 负责把原始物理行映射为带行号的记录,冲突重建由此取输入;history.repair_index_duplicates 复用相同分隔规则,避免将终止 LF 回写为额外空行。_find_monitor_poll_turn 倒序寻找指定 Todo/target 的监控轮次,_actual_work_window 汇总周期报告的实际工作窗口;这两个只读读者现在不会在 Unicode 分隔字符处打断一条 JSON 记录。测试使用真实临时索引文件覆盖 Unicode 值、普通记录、内部空行、终止 LF、LF/CRLF/无终止符的重复项修复,以及冲突重建后的备份和索引内容。

正向路径上,含 U+0085/U+2028/U+2029 的单条记录能被完整解析并用于监控及报告;反向路径上,重复项修复再执行一次不会继续增长空行,冲突重建保留备份并只输出预期记录。上一轮指出的尾部空哨兵问题在此 head 已修复;此前提到的监控参数回归也不在当前完整 diff 中。

对主干的风险

没有发现阻断性问题。 精确 head 的索引/历史/配额定向套件 41 passed,报告与监控相关套件 55 passed;我还用真实临时文件分别验证了三种 Unicode 值在索引读取、监控命中及报告窗口中的结果。Ruff 与 git diff --check 通过。首选的 uv editable build 被本机磁盘空间不足(ENOSPC)挡住;随后使用现有兼容虚拟环境、明确设置精确 head 的 PYTHONPATH,核对 loopx.__file__ 指向该工作树并完成上述 96 个测试。这个构建中断是本地环境限制,不是 PR 回归;完整仓库测试和远端 CI 未纳入本次结论。

相邻的 project_map.latest_operator_gate 仍以 splitlines() 读取同一索引;正常情况下它可从每轮 JSON 文件回退,因此不阻断本次四个读者的修复。我用真实临时文件验证:只有索引且门禁记录含 U+0085 时返回 None,加入对应 JSON 文件后可读回审批。这是一个可单独收敛的 P2 清理点:该相邻读者后续也应按物理 LF 读索引,并覆盖“只剩索引”的读回。未来相关重构的边界是统一物理记录语义,而不是增加第二套索引状态。

语义与 CI 对齐

本 PR 复用已有的 LF 分隔 JSONL 契约,没有新增状态词汇或改变配额决定权;写入端仍使用 ensure_ascii=False,因此读入端修正与持久格式一致。本次能力计划 wait_for_ci=false,所以远端 CI 不在此次评审证据中;本地受影响的定向检查如上。

我的整体评价

APPROVE。 当前变更针对真实可复现的数据丢失,四处目标读者及两个写回路径的语义一致,前次空行回归已有真实文件测试守住。长期看,正确保留监控和历史记录避免后续调度/审计失真;用户侧的报告与监控读回也恢复一致。剩余的 project_map 读者和未运行的全量检查是明确的边界,不构成当前精确 head 的阻断。本评审只给出代码评审结论,不授权合并。

English verdict: APPROVE - exact head 7123bbe preserves Unicode data in the four scoped JSONL readers and fixes the previous trailing-LF rewrite regression; 96 focused tests, real-file Unicode probes, Ruff, and diff checks passed. The adjacent project-map reader is a non-blocking follow-up.

@huangruiteng
huangruiteng merged commit f7f50b8 into loopx-project:main Sep 29, 2026
26 of 28 checks passed
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