Skip to content

fix(chat): keep completed-history pages independent across Goals - #6496

Open
hhyykk wants to merge 2 commits into
loopx-project:mainfrom
hhyykk:codex/completed-history-lock-isolation
Open

hhyykk wants to merge 2 commits into
loopx-project:mainfrom
hhyykk:codex/completed-history-lock-isolation

Conversation

@hhyykk

@hhyykk hhyykk commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Goal And Delivered Outcome

Loading one Goal's completed history holds the server-wide page-cache lock, so another Goal's already-cached page waits for the unrelated backend read. Prepare the snapshot and encoded size outside the mutex; keep cursor validation, expiry, eviction and publication inside it. Cached pages can now finish while the other source remains held.

  • Basis: the reproduced cross-Goal wait and accepted repair; no separate written specification. Existing source-region isolation guidance: docs/architecture/rfcs/goal-workspace-surface-v0.md.
  • Intended base: main, validated against 0b4df8a117a96176651aa6d019209fbdefb1539e.
  • Five-minute retention starts when the prepared snapshot is published, so a slow load cannot immediately expire its own cursor. The existing 40-row pages, scope checks, eight-snapshot and 16 MiB retained encoded-data bounds remain.

Author Declaration

  • Written by: model_agent, OpenAI Codex (GPT-6 family).

Implemented against

Accepted criterion Disposition Owner Verification
Cached and independent first pages progress while another loader waits implemented CompletedTodoPages.page Two regression cases fail before and pass after
Concurrent publication preserves cursor scope, expiry and cache budgets implemented Existing page cache Concurrent count/byte budgets, publication-clock expiry and existing pagination tests
The existing task journey uses the repaired owner and recovers from read failure implemented CompletedTaskLane → production HTTP/cache Packaged completed-history-isolation scenario, including 503 recovery

Before / After Comparison

Pinned baseline: 9493e6fffa34a61e2fd0056d4f048bc3bcfd27f2. Candidate product source matches b052979293b873235bd50f1b8cdd3481c0b662dc. Measurements preceded commit/rebase; the only new-base change was an unrelated work-map browser assertion. Product source hashes match all formal trials.

Same real ChatHTTPServer, Markdown Todo reader and disposable data. Goal B has 85 completed tasks and reads its cached 40-row second page. Three alternating pairs per workload (before→after, after→before, before→after), three observations per trial: nine samples per arm per workload, 36 total. Backend Node 22.22.3, Python 3.11.6. Timings cover B's HTTP request through JSON decoding.

Workload while Goal A loads B's median before B's median after B returns during A's held read, before → after
A has 85 tasks; its filesystem read is held for a fixed 400ms 482.3ms 1.4ms 0/9 → 9/9
A has 4,087 tasks; no added delay 1326.7ms 37.9ms N/A

Natural-case ranges: 1280.0–1653.6ms before, 1.3–64.9ms after. All rows, order and totals were correct; persisted input hashes stayed unchanged; no POST or model Turn occurred. An initial run overlapping repository indexing was retained as a pilot and excluded in full; the fixed protocol was rerun after that activity ended.

Conclusion: unrelated cached pagination stops waiting for the page-cache mutex held by a large/slow source. This does not accelerate that source itself or establish production latency, provider throughput or memory admission. Aggregate comparison and source hashes.

Validation

  • Tested revision: b052979293b873235bd50f1b8cdd3481c0b662dc; finished; inputs: synthetic, public_fixture.
Check kind Result Evidence / limitation
unit / real_backend passed 28 tests, direct interpreter exit 0: python -m pytest tests/test_chat_completed_todos.py tests/test_chat_explore_results.py tests/test_managed_research_team.py; legacy Markdown, canonical File/SQLite, accepted reports and Explore reads
regression_parity passed Both independent-read cases fail on the old owner; five new regression cases pass on final head
real_entrypoint passed Dashboard/packaged Chat build and source verification; no frontend product-source change during rebase
integration passed Packaged task view + production completed-history HTTP/cache; original backend times out waiting for the next page; final head reads while A remains held, retains visible rows after 503, retries to all 85 tasks and starts no Turn
static passed Ruff, diff whitespace, code/test/docs guard self-review; no independent-review claim
CI pending Required PR checks have not completed; no merge-readiness claim

Setup-only dependency and retry-expectation failures were corrected and retained in local records. The final direct-interpreter run passed all 28 tests and exited 0; the packaged scenario also exited 0.

Frontend / Visual Evidence

Existing Tasks completed lane; no layout, navigation, settings or CTA change. Unrelated workspace APIs use the established public browser fixture; the completed-history endpoint and persistence are real. Chromium 145.0.7632.6. Synthetic data only.

  • Before, B waits while A's source is held (desktop 1512×982):

Before: completed history waits

  • After, B's next page is readable before A is released (same desktop):

After: independent completed page

  • After503 recovery through the final saved task (mobile 390×844):

After: recovered completed history on mobile

Attention/craft review: the existing task cards, loading/error copy, retry control and virtualized lane remain. No new grouping, motion, glyph or color treatment.

Scope And Continuation

Complete within the shared page-cache isolation repair. The same owner serves completed tasks, accepted reports and Explore results. No new API, configuration, store or execution authority. Retained-cache budgets remain bounded; peak simultaneous loader/encoder memory, production soak, PostgreSQL, real provider execution and installed/Windows desktop are unqualified. No deployment or merge.

Type of Change

  • Bug fix / performance improvement
  • Test update

LoopX Area

  • Public docs or presentation surface (dashboard)

Technical Direction

Existing Python presentation/IO cache, roadmap S5/S10 fault isolation. TypeScript domain decisions and canonical providers remain their current owners. Future-facing pass: narrow this shared owner instead of adding per-Goal lock maps or a second cache. Shared-authority fixture impact: N/A; no promotion, routing or compatibility-projection change.

Boundary Checklist

  • Diff, body and attachments contain no private state, credentials, raw logs or machine paths.
  • Scoped to the reproduced cache-lock problem; no maintainer benchmark duplicated.
  • Synthetic before/after visual evidence and validation limits included.
  • Every commit has a DCO Signed-off-by trailer.

Signed-off-by: aihop <4408344+hhyykk@users.noreply.github.com>
Signed-off-by: aihop <4408344+hhyykk@users.noreply.github.com>

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

Reviewer: model_agent; model=gpt-6.1-sol; provider=OpenAI; declaration_source=runtime_reported; reasoning_effort=xhigh

Reviewed exact head: 6496@b052979293b873235bd50f1b8cdd3481c0b662dc.

动机

在一个 Goal 中翻阅完成历史、同时另一个 Goal 仍在读取数据的用户。 旧共享缓存把源读取放在全局锁内,另一 Goal 的已缓存下一页也会等待;本头先在锁外准备数据,独立分页可以先返回。 真实 HTTP 对照与打包页面验证支持锁隔离和 503 后重试,新增浏览器夹具却在符号链接临时目录下失去慢读同步。 先修正新增夹具的路径身份比较并在别名临时目录下复验;保留当前源码隔离结果和未完成的合并门禁。 不证明生产延迟、并发准备数据的峰值内存、PostgreSQL、安装态或 Windows 桌面;其他工作区 API 使用合成夹具。

改动思路

继续使用原 CompletedTodoPages,只缩短其互斥区:首次读取和编码大小计算可以在锁外完成;检查旧 cursor、清理过期项、驱逐和发布新快照仍在同一锁内。无需每 Goal 的锁表或第二份缓存。保留既有缓存/回执 owner 和类型化消费者;这次问题不需要新增框架。当前结论限定为源码与真实本地读取/回执链路;夹具、跨版本部署、安装态和长期效果按实际证据分别判断。

这属于既有 Python 呈现/IO 缓存,Todo 权威和 TypeScript 页面决定没有迁移。依据变更前 docs/architecture/rfcs/goal-workspace-surface-v0.md,不可变版本 0b4df8a117a96176651aa6d019209fbdefb1539e,核验 2. Problem and motivation 中的来源隔离不变量,以及 5.5 All-work rail 的原生历史分页边界;本 PR 不完成整个工作区 RFC。

具体改动

  1. loopx/chat_completed_todos.py:88 的 page 在无 cursor 时先 load()、算编码字节并拒绝超大结果,再进入锁。已有 cursor 不重新读源;scope、offset、过期与缓存发布的原检查仍串行执行。五分钟 TTL 从实际发布开始,慢读取不会立即耗尽新 cursor 的寿命。
  2. 同一缓存服务完成任务、已验收报告和 Explore 结果;三个真实消费者均已检查。40 行页、8 份快照及 16 MiB 保留编码数据上限不变;这些上限没有变成并发 loader 的峰值内存准入保证。
  3. 新增五个并发/TTL 用例覆盖有/无缓存的独立读、并发数量/字节预算和发布时钟;原完成历史、Explore 与 managed-research-team 套件合计 28 passed、0 skipped。
  4. 新的浏览器场景接入既有 catalog,另用真实 ChatHTTPServer/Todo 文件提供持有源读取、503 与恢复控制。所有五个路径都已读完;新增可复用验证有价值,但路径同步存在下述平台缺陷。

独立验证使用同一私有夹具及真实 HTTP/Markdown:不可变基线的 B 第二页在 A 持有源读取期间保持 pending,释放 A 后返回;本头在同样状态下已返回正确 40 行。随后从本头重新构建的 Chat 页面实测,A 仍被持有时能看到 B 的第 44 项;503 后已读行仍可见,重试到第 0 项、共 85 项,输入哈希不变且没有 POST、Turn 或 action apply。桌面保留 6 个虚拟 DOM 行;390×844 页面可读最后一项、无横向溢出。其他工作区 API 是合成夹具,摘要计数并非此次真实历史后端的资格化内容。

上述正向隔离验证在私有夹具中把临时根路径规范化,两臂一致;提交的原夹具另行原样执行,不能把私有修正冒充本头已修复。作者公开比较材料已读,cache owner 的 base/head 哈希与本次 source 相符,但其毫秒值不是我的独立性能测量,不据此宣称生产 SLO。

风险 canary 的 5 个直接检查与 6 个选中检查均无阻塞执行失败,含全树 semantic 和显式路径边界检查;开发期 advisory 先于全树检查。整体合并门禁仍报过期的精确范围质量回执,没有拿旧回执证明本头可合并。使用显式关闭 Node compile cache 的环境;未查询或等待远程 CI。

对主干的风险

[P2] 让新增慢读夹具比较同一个规范路径。 examples/personal-workspace-browser/completed_history_fixture.py:15 保留 Path(directory) 的字面路径,第 37 行却用它与生产读取端已经 resolve 的 state.md 路径比较。临时目录经过符号链接时,两者指向同一文件却不相等,例如 macOS 常见的 /var 别名。

以合成的符号链接 TMPDIR 直接启动未修改的提交夹具,真实 slow Goal GET 在 release 前就退出成功、返回 85 行,而且没有发出 slow-read。生产解析器返回的实际路径与夹具目标 resolve 后相同、字面值不同。新增 JS 场景等待这个事件,因而会超时,而不是验证宣称的锁隔离;28 项 Python 测试没有覆盖这个入口。

最小修复是构造 registry/目标路径前对临时根统一 .resolve(),或在持有回调中比较规范路径身份,并补一个 symlink TMPDIR 回归。保留真实持有、独立分页、503 重试与零写入断言,不通过延长超时或跳过平台分支掩盖同步缺失。我的私有规范路径对照已经证明核心缓存修复可工作,这不替代提交修复和原场景复验。

并发准备数据会提高在途内存峰值,这次只证明保留缓存预算与来源隔离;没有给出生产 soak、物理内存准入或自然负载吞吐结论。手机完整 DOM 的既有响应式行为也未被这处 Python 改动改变,不将桌面虚拟化断言扩大成手机 DOM 上限承诺。

我的整体评价

当前精确头 REQUEST_CHANGES:原缓存的锁隔离修复有真实入口支持,新增验证方向也合适;阻塞是新夹具在别名临时目录下不能进入其关键同步点。前瞻检查支持保持一个 cache owner、只把准备过程移出锁;无需新增锁管理框架。修正夹具并按新头复验后再给结论,安装态、内存峰值与合并门禁仍单独保留。

English verdict: REQUEST_CHANGES - b052979293b873235bd50f1b8cdd3481c0b662dc fixes the real shared-cache wait and passes 28 focused tests, but the added browser backend compares a lexical temp root with resolved production paths. A symlinked TMPDIR makes its slow GET finish without the synchronization event, so the new scenario times out. Canonicalize the fixture path and retain the real isolation/recovery checks.

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