Repository navigation
fix(lark): accept microsecond comment timestamps - #6084
mikamikasuki wants to merge 2 commits into
Conversation
Signed-off-by: mika <211269698+mikamikasuki@users.noreply.github.com>
loopx-agent
left a comment
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent | gpt-6.1-sol | OpenAI | runtime_reported | reasoning_effort=xhigh
REQUEST_CHANGES — [P2] Preserve the typed provider error for non-decimal digit strings.
Exact reviewed head: 6084@73f773421bf9201516bd1ebb677f6066e209af17; immutable base 07eb7a1288e8a33995493f9d50475f2664fcbf2e. No approval inherited, CI not queried/polled/waited for.
动机
通过飞书文档评论收集任务的用户，以及读取评论页和嵌套回复的 Connector 调用者。
同一条16位微秒评论原来被误判为无效时间；head能保留六位小数并捕获到持久 inbox。但包含上标数字的畸形时间现在泄漏原始 ValueError，丢失既有内容无关的 provider 错误码。
实际默认 OS subprocess 调用合成 lark-cli 原始 JSON，评论页/嵌套页及真实 inbox 分别读回微秒精度。22个原始输入中10项页面完全相同、12项有明确变化；其中两项上标数字从 typed comment_timestamp_invalid 变为原始 ValueError，正常改正后捕获恢复。
本次只验证现有评论时间转换、provider 调用和本地持久捕获；不调用真实飞书、不新增权限、自动接入或评论发布。
改动思路
单位选择和 provider 错误归一属于既有 Python Lark adapter；小改动可修复精度，无需新 capability 或通用时间服务。必须同时保留原转换错误边界。
当前PR边界：现有评论时间转换、两种分页、原 typed 错误及本地持久捕获。
The callback parser supports 13/16-digit fields but has a different error contract and no seconds route. Reuse of the integer divmod approach is sensible; copying its isdigit/int gap is unsafe. Keep provider-specific sanitization in the existing owner; a new generic framework is unnecessary.
最小修复：Keep numeric conversion inside the existing typed exception boundary (or use an explicitly decimal/ASCII grammar with the same error), and add page_reader/capture regressions for isdigit-but-not-int inputs. Preserve seconds/milliseconds/microseconds precision and no durable capture on invalid input. 有界后续重构检查已做：最近owner能承载修复，保留不同callback/provider错误及支持精度合同；不要求新通用框架或无关TypeScript迁移。
具体改动
规格先读公开问题 #6083（snapshot 2026-10-10T06:49:34Z），再读固定revision的既有合同，然后检查全部diff及调用者。Expected behavior在真实生产入口已实现；既有内容无关provider错误未满足，不把作者声明或测试数作为验收。
Criterion Existing provider failure boundary: not_met — int(token) is outside the protected conversion. page_reader and capture raise raw ValueError containing the input instead of LarkDocumentCommentProviderError("comment_timestamp_invalid"); callers lose the stable provider error contract.
关键代码讲解
_epoch_rfc3339(loopx/extensions/lark/document_comment_provider.py:287)：按宽度选择单位并保留余数，但未保护 int 转换。_events_from_replies(loopx/extensions/lark/document_comment_provider.py:639)：两种分页调用同一时间转换，再生成既有事件。page_reader(loopx/extensions/lark/document_comment_provider.py:874)：既有 source/权限及 cursor 路由，capture 后独立读回。
全diff两文件：生产15行新增/9行删除，其中 import 整理2行；测试5行新增，仅验证private helper微秒happy path。新增按13/16位识别单位、整除和余数重建小数，支持最多10位秒；回归位于isdigit与int转换之间。
阻塞问题
[P2] Preserve the typed provider error for non-decimal digit strings
触发：A comment reply create_time is "²" or "170000000²"; str.isdigit() accepts it, but int() cannot parse it.
实际结果：int(token) is outside the protected conversion. page_reader and capture raise raw ValueError containing the input instead of LarkDocumentCommentProviderError("comment_timestamp_invalid"); callers lose the stable provider error contract.
修复与回归：Keep numeric conversion inside the existing typed exception boundary (or use an explicitly decimal/ASCII grammar with the same error), and add page_reader/capture regressions for isdigit-but-not-int inputs. Preserve seconds/milliseconds/microseconds precision and no durable capture on invalid input. Run the actual page_reader and capture_document_comment_provider_page with create_time="²" and "170000000²": expect typed content-free comment_timestamp_invalid, no accepted event; replace with a valid microsecond timestamp and verify persisted event precision.
对主干的风险
22个页面输入10项全同/12项变化：微秒三个样本与16位最大样本新增接受，毫秒小数保留是预期变化；signed/negative及11/12/14位输入现在拒绝，已明确披露。真正阻塞是两项上标数字的异常类型/内容变化。实际捕获拒绝没有持久事件，正常改正后accepted_count=1；两种分页均复现同一问题。
42 passed in 0.18s on base; 43 passed in 0.78s on head. Ruff and diff-check pass. Current-diff semantic advisory precedes canary: 5 direct and 10 selected checks pass, zero failures/skips; CI not consulted.
作者先前Ruff记录不是当前失败归因；本轮同一source环境的base/head Ruff均通过，不猜测历史原因。上述风险在测试与canary全绿时仍通过实际入口复现，不能用green检查消除。初始评审夹具的命令匹配及读回异常记录问题已保留并修正；最终相同脚本在B/H完整运行。没有真实飞书、活跃Goal或外部服务写入，也未观察打包UI；本PR没有新UI操作入口。
语义与 CI 对齐
这里复用既有provider错误词汇和本地external-input单位合同，没有新actor/permission、substring denylist或平行状态分类。int(token) is outside the protected conversion. page_reader and capture raise raw ValueError containing the input instead of LarkDocumentCommentProviderError("comment_timestamp_invalid"); callers lose the stable provider error contract. 这违反固定revision的既有合同，需要上面的最小修复及实际caller回归；不以未来RFC或CI状态作为阻塞依据。没有新的opt-in/default-off承诺；原permissions、claim、effect和cursor authority仍独立。机器拒绝与advisory建议不能混称。
我的整体评价
REQUEST_CHANGES。局部正向修复成立，生产机制规模合理、归属清楚；完整切片仍不能通过，因为实际负向/后续工作合同出现回归。long_horizon与user_experience均判为regression：int(token) is outside the protected conversion. page_reader and capture raise raw ValueError containing the input instead of LarkDocumentCommentProviderError("comment_timestamp_invalid"); callers lose the stable provider error contract.
单位选择和 provider 错误归一属于既有 Python Lark adapter；小改动可修复精度，无需新 capability 或通用时间服务。必须同时保留原转换错误边界。 现有评论时间转换、两种分页、原 typed 错误及本地持久捕获。 修复后重跑同一实际入口和上面的回归，并对新head重新评审。范围内没有新权力或自动激活；未执行merge、dismiss或源码修复。历史经验只作为outcome-first分析建议采用，不能据此宣称记忆带来了可测效果。
English review
Motivation. Users collecting work from document comments need supported epoch seconds, milliseconds and microseconds to become accurate, durable events. The original 16-digit case was rejected; the new provider normalizer preserves its fractional part through actual capture and a separate filesystem inbox readback. Issue #6083 was read before the implementation; the accepted contract at docs/reference/extensions.md@07eb7a1288e8a33995493f9d50475f2664fcbf2e supplies the unchanged negative/continuation boundary. This reviews the entire exact head, not only the added helper test.
Implementation and ownership. The integer quotient/remainder approach and width recognition belong in the existing Lark provider adapter. The callback parser has different supported widths and error semantics, so no generic framework or blind helper substitution is required. Both comment and nested-reply pages flow through the same normalizer. The full diff includes import cleanup,15 production additions/9 deletions and5 test additions. The future-facing ownership pass keeps these existing boundaries and requests the smallest related correction.
Blocking finding and real reproduction. A comment reply create_time is "²" or "170000000²"; str.isdigit() accepts it, but int() cannot parse it. int(token) is outside the protected conversion. page_reader and capture raise raw ValueError containing the input instead of LarkDocumentCommentProviderError("comment_timestamp_invalid"); callers lose the stable provider error contract. Keep numeric conversion inside the existing typed exception boundary (or use an explicitly decimal/ASCII grammar with the same error), and add page_reader/capture regressions for isdigit-but-not-int inputs. Preserve seconds/milliseconds/microseconds precision and no durable capture on invalid input. Run the actual page_reader and capture_document_comment_provider_page with create_time="²" and "170000000²": expect typed content-free comment_timestamp_invalid, no accepted event; replace with a valid microsecond timestamp and verify persisted event precision.
Validation and full-diff risk. The same immutable harness executes the actual default OS subprocess path with raw synthetic lark-cli JSON, both pagination routes, permission probing and a real filesystem inbox.22 inputs yield10 identical complete pages and12 enumerated deltas. Microsecond acceptance and millisecond precision are intended; signed/negative and11/12/14-digit forms are now rejected, which is disclosed. Superscript-digit conversion is the unintended error-contract regression. Invalid capture writes no event, and a corrected ordinary input succeeds. No live Lark account or remote write is claimed. 42 passed in 0.18s on base; 43 passed in 0.78s on head. Ruff and diff-check pass. Current-diff semantic advisory precedes canary: 5 direct and 10 selected checks pass, zero failures/skips; CI not consulted. Initial reviewer-fixture failures are retained and corrected; current production-counterexample evidence remains failing. No active Goal, paid model, PostgreSQL or packaged UI validation is claimed.
Overall judgment. The original local defect is fixed and the mechanism is proportionate, but sustained continuation and user experience regress at the demonstrated negative boundary. Preserve the existing typed error/UTC vocabulary and repair the named existing owner before re-review. No opt-in/default-off claim, actor authority, permission or automatic activation is added. CI and merge authority are separate; no merge was performed.
English verdict: REQUEST_CHANGES — 6084@73f773421bf9201516bd1ebb677f6066e209af17. Positive issue reproduction passes; the actual provider error counterexample above blocks this head despite passing local suites and canary.
| if precision is None: | ||
| raise LarkDocumentCommentProviderError("comment_timestamp_invalid") | ||
| numeric = int(token) | ||
| seconds, remainder = divmod(numeric, precision) |
There was a problem hiding this comment.
[P2] Preserve the typed provider error for non-decimal digit strings
A comment reply create_time is "²" or "170000000²"; str.isdigit() accepts it, but int() cannot parse it. int(token) is outside the protected conversion. page_reader and capture raise raw ValueError containing the input instead of LarkDocumentCommentProviderError("comment_timestamp_invalid"); callers lose the stable provider error contract.
Keep numeric conversion inside the existing typed exception boundary (or use an explicitly decimal/ASCII grammar with the same error), and add page_reader/capture regressions for isdigit-but-not-int inputs. Preserve seconds/milliseconds/microseconds precision and no durable capture on invalid input.
Regression: Run the actual page_reader and capture_document_comment_provider_page with create_time="²" and "170000000²": expect typed content-free comment_timestamp_invalid, no accepted event; replace with a valid microsecond timestamp and verify persisted event precision.
|
Hi @mikamikasuki, the DCO If the log confirms a missing |
loopx-agent
left a comment
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent | gpt-6.1-sol | OpenAI | runtime_reported | effort=max
REQUEST_CHANGES — [P2] 新修复提交缺少 DCO 签署。 直接读取提交对象确认 4c4fb1098efb50f3c68984a615d253f621797954 没有 Signed-off-by；原提交已签署。请在原分支 git commit --amend -s 后用 --force-with-lease 推送，并核验全部 PR 提交。此结论来自提交内容，没有查询 CI。
Exact reviewed head: 6084@4c4fb1098efb50f3c68984a615d253f621797954；base 07eb7a1288e8a33995493f9d50475f2664fcbf2e。
动机
受影响的是把飞书文档评论收进任务 inbox 的用户，以及读取评论页和嵌套回复的调用者。
一条带六位小数的微秒时间原来被误当作更大的秒值，导致整页评论被拒绝；本次修复使评论保留准确时间，并能持久保存后继续处理。
当前版本通过真实子进程读取合成评论，两种分页均保留微秒精度；旧评审发现的畸形数字异常也已消除，但新增提交缺少仓库必需的 DCO 签署。
范围限于现有评论时间转换和错误边界；没有启用新连接、发送真实飞书消息、改变权限或验证已安装客户端。
功能验证已通过；作者仍须给当前新增提交补齐 Signed-off-by，再提交新 head 供核验。
改动思路
时间格式属于既有 Lark provider，权限与持久捕获继续由原通用 owner 负责；没有必要增加时间服务、新状态或额外配置。
当前 PR 边界是评论时间转换、两种分页、稳定错误和本地捕获；当前还需补齐新增提交的 DCO。
按完整 base→head 两文件评审，再单独核对上次 73f 之后的修复。整数商和余数保留精度；ASCII 检查与受保护的转换使畸形数据回到原错误合同。相邻 callback parser 没有秒路由且错误语义不同，适合复用算法，没必要强合 helper。现有已授权连接无需新增配置、重填信息或确认轮次。
具体改动
全部两文件 +27/-6：生产 +15/-6，包括时间转换和 import 整理；测试 +12，新增微秒正例与非 ASCII 数字拒绝。没有新模块、协议版本、状态 owner 或命令。独立规范为 #6083，固定快照 issue-6083-body-sha256:596936590b96ac2053e4fec8a5e7d0ed70144703dcd67b3fecb9e8fdc0329a64，在读 diff 前取得；条款 Expected behavior：implemented，支持的秒/毫秒/微秒均产生独立预期的 RFC3339。既有内容无关错误另按 docs/reference/extensions.md@07eb7a1288e8a33995493f9d50475f2664fcbf2e 核验。
关键代码讲解
_epoch_rfc3339先拒绝布尔及非 ASCII 数字，再按宽度选单位；int转换仍在 typed 异常边界内，UTC 时间加余数不会丢掉六位精度。_events_from_replies为普通评论和嵌套回复生成原事件；无效时间在任何持久捕获之前报错，原事件身份与权限含义不变。page_reader沿既有私有 cursor 选择分页，不增加执行或回复权；generic capture 在权限满足后才调用它。
对主干的风险
同一脚本通过默认 OS subprocess 读取合成原始 JSON，对 base/head 各执行 48 项页面观察：22 项完全相同，26 项是可列出的单位、精度或格式变化。支持微秒、保留毫秒小数是预期；符号、负值、不支持宽度及非 ASCII 数字现在明确拒绝。无 opt-in/default-off 承诺，也没有新状态分类或机器义务。
两种分页各验证一次正常捕获与两种畸形输入：错误均为内容无关的 comment_timestamp_invalid，错误前没有事件；替换正常微秒值后原 cursor 重试 accepted=1。独立读取六份实际事件 JSON，时间均为 2023-11-14T22:13:20.123456Z。权限不足控制没有调用 provider 子进程。外部数据是合成 Lark 接口，持久化和子进程为真实路径；没有冒称真实飞书验收。
当前 44 项相关测试、base 42 项、Ruff、diff-scoped semantic advisory、原生 premerge 5 项 direct 与 10 项 selected 全通过。旧 raw ValueError 阻塞已经解决。当前仍有独立的 DCO 必需项失败：44 个测试和 canary 不检查提交签署，不能替代 CONTRIBUTING.md 的 DCO 要求；修复只涉及原提交签署，之后重读新 head。
我的整体评价
局部功能及错误恢复已验证，机制规模合理，沿既有 provider 归属，当前交付判断为 justified_increment。long_horizon 与 user_experience 在此边界均 improved：正常评论能留下可继续处理的结果，错误不泄露内容、改正后无需手动清状态；没有测量模型或长期成本净益。兼容性保留既有 RFC3339 与 typed error，不引入通用框架；本地 external_input 词汇复用现有 owner。当前唯一请求修改项为新增提交的 DCO，补签后对新 head 核验即可；合并与安装资格独立。
English verdict: REQUEST_CHANGES — 6084@4c4fb1098efb50f3c68984a615d253f621797954. The prior parser error regression is resolved through actual subprocess/capture/recovery paths, and focused tests plus premerge pass. The new commit itself lacks the mandatory Signed-off-by trailer; amend/sign it and verify the new exact head. CI was not consulted.
This branch has not been deployed
2 participants
Goal And Delivered Outcome
Author Declaration
Implemented against
Scope And Continuation
Validation
Frontend / Visual Evidence
Type of Change
LoopX Area
Technical Direction
Shared-authority RFC fixture impact
Boundary Checklist