docs(base): stabilize continuous-period sums - #2262
huarenmin13 wants to merge 1 commit into
Conversation
github-actions
Bot
added
domain/base
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe Lark Base data analysis SOP adds rules for consistent statistical scope, date-field selection, ratio denominators, zero-denominator handling, complete time-bucket output, and trend conclusions. ChangesStatistical scope updates
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/lark-base/references/lark-base-data-query-guide.md`:
- Line 23: Update the rate-denominator guidance in the calendar-period query
instructions to make it independent of numerator status scope: use all records
in each requested bucket as the denominator whenever a denominator is not
explicitly specified, even when the numerator has a status filter. Restrict a
closed-only denominator to requests that explicitly ask for it, and align the
wording with the rule in SKILL.md.
In `@tests/cli_e2e/base/base_skill_contract_test.go`:
- Around line 47-61: Expand the contract assertions in the test around the
existing skill, SOP, and guide checks to cover the structured-error gate,
prohibition of unauthorized `--as bot` fallback, formula echoing, empty-bucket
handling, and half-open datetime boundaries. Add direct assertions for each
changed clause, and use strings.Index with require.Less to verify the
user-credential attempt appears before authorization recovery.
- Line 24: Update the assertion in the base skill contract test to match wording
actually present in SKILL.md, replacing the absent “文件导入转 lark-drive” substring
with an exact existing file-import phrase such as the local-file/Base import or
lark-drive import/export wording.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dff154a1-f641-4d14-8c5e-8d58157c495a
📒 Files selected for processing (7)
shortcuts/base/base_data_query.goshortcuts/base/base_shortcuts_test.goshortcuts/base/record_list.goskills/lark-base/SKILL.mdskills/lark-base/references/lark-base-data-analysis-sop.mdskills/lark-base/references/lark-base-data-query-guide.mdtests/cli_e2e/base/base_skill_contract_test.go
|
|
||
| skill := string(content) | ||
| require.Contains(t, skill, "文件导入/导出转 lark-drive") | ||
| require.Contains(t, skill, "文件导入转 lark-drive") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix the file-import wording assertion.
Line 24 expects 文件导入转 lark-drive, but skills/lark-base/SKILL.md contains 把本地文件导入成 Base and 本地文件与 Base 之间的导入/导出转 \lark-drive`` instead. The exact substring is absent, so this test fails deterministically.
Proposed fix
- require.Contains(t, skill, "文件导入转 lark-drive")
+ require.Contains(t, skill, "把本地文件导入成 Base")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| require.Contains(t, skill, "文件导入转 lark-drive") | |
| require.Contains(t, skill, "把本地文件导入成 Base") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/cli_e2e/base/base_skill_contract_test.go` at line 24, Update the
assertion in the base skill contract test to match wording actually present in
SKILL.md, replacing the absent “文件导入转 lark-drive” substring with an exact
existing file-import phrase such as the local-file/Base import or lark-drive
import/export wording.
| for _, want := range []string{ | ||
| "先按本 skill 的路径尝试 `--as user`", | ||
| "不要因为看到 `/base/` 链接就预先运行 `auth login`", | ||
| "原始 `datetime` / `created_at` 字段", | ||
| "未指定比例分母时，默认用请求时间桶内的全部记录", | ||
| } { | ||
| require.Contains(t, skill, want) | ||
| } | ||
| require.Contains(t, sop, "完整正确性契约统一见 [lark-base-data-query-guide.md]") | ||
| require.Contains(t, guide, "copy the user's requested measure, date field, status scope, and ratio denominator") | ||
| require.Contains(t, guide, "datetime `isGreater`/`isLess` are strict") | ||
| require.Contains(t, guide, "A zero denominator is “no data”, not 0%") | ||
| require.Contains(t, guide, "prefer an original `datetime` or `created_at` field") | ||
| require.Contains(t, guide, "use all records in each requested bucket as the denominator") | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the complete identity and analysis contract.
These assertions cover only selected substrings. They do not verify the structured-error gate, the prohibition on unauthorized --as bot fallback, formula echoing, empty-bucket handling, or half-open datetime boundaries. Presence checks also do not verify that the user-credential attempt occurs before authorization recovery.
Add assertions for each changed clause. Use strings.Index with require.Less for the identity ordering.
As per coding guidelines, contract tests must assert the changed behavior directly so reverting the implementation causes a test failure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/cli_e2e/base/base_skill_contract_test.go` around lines 47 - 61, Expand
the contract assertions in the test around the existing skill, SOP, and guide
checks to cover the structured-error gate, prohibition of unauthorized `--as
bot` fallback, formula echoing, empty-bucket handling, and half-open datetime
boundaries. Add direct assertions for each changed clause, and use strings.Index
with require.Less to verify the user-credential attempt appears before
authorization recovery.
Source: Coding guidelines
huarenmin13
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/lark-base/references/lark-base-data-analysis-sop.md`:
- Around line 21-23: Update
skills/lark-base/references/lark-base-data-analysis-sop.md lines 21-23 so
candidate date-field selection and non-empty coverage validation apply only when
the request includes date filters, time dimensions, or calendar-range semantics;
non-temporal analyses must not force a time field or add filters. Update
skills/lark-base/SKILL.md line 109 so time-field traceability is required only
for temporal criteria, while non-temporal analyses explicitly report that no
time field was used.
In `@skills/lark-base/SKILL.md`:
- Line 43: Update the Base/Wiki URL resolution rule to include a
workflow-specific lookup when block_type is workflow and block_name does not
match the user’s target: use +workflow-list or reference the existing workflow
lookup rule. Keep the current type-specific branches for data tables and
dashboards unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4180760e-0d5f-41d7-a2f8-06b359077d8d
📒 Files selected for processing (4)
skills/lark-base/SKILL.mdskills/lark-base/references/lark-base-data-analysis-sop.mdskills/lark-base/references/lark-base-data-query-guide.mdskills/lark-base/references/lark-base-data-query.md
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/lark-base/references/lark-base-data-query-guide.md
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@5f28fe74fef3fe8b099e14d00f2a4b5ff3830833🧩 Skill updatenpx skills add huarenmin13/cli#auto-research-sync/01KZD9G82QWMHDT8K47GNZNZ50/mr-1404-776defac -y -g |
dd0d647 to
b31df93
Compare
huarenmin13
changed the title
b31df93 to
1835de5
Compare
huarenmin13
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/lark-base/references/lark-base-data-analysis-sop.md`:
- Around line 27-29: 补充该数据分析 SOP
的完整无数据契约：在最终输出前为用户要求的每个时间桶执行缺失桶补全，即使查询未返回记录也要输出该桶；桶级或总体分母为 0 时统一输出“无数据”，不得将其当作 0
参与趋势分析。明确仅对有效分母桶计算趋势，并规定部分桶有效时总体分子、分母、比率及趋势结论的处理方式；如已有相关流程参考，链接该引用。
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f6f82db6-1905-4c6c-87b3-ee9efb1bc7fd
📒 Files selected for processing (1)
skills/lark-base/references/lark-base-data-analysis-sop.md
huarenmin13
changed the title
yballul-bytedance
left a comment
yballul-bytedance
left a comment
There was a problem hiding this comment.
Reviewed the final net diff. This is a docs-only update to the Base data analysis SOP, and the generalized metric/ratio/time-bucket contracts look scoped and consistent with the reference structure. Approval is based on the current documentation diff; CI still needs to finish separately.
0fc5d0a to
5f28fe7
Compare
|
|
huarenmin13
changed the title
Narrow the change to reusable correctness gaps in continuous calendar-period sums; runtime behavior is unchanged and the always-loaded SKILL.md is untouched. - Use adjacent half-open calendar intervals [start, next_start) and the actual +record-list lower-bound capability. - Restrict +record-list datetime range conditions to strict > / <, and keep a safe strict upper-bound example while removing the unsafe prior-day-millisecond lower-bound workaround. - Route exact local-calendar boundaries through the +data-query date dimension recovery path or a complete NDJSON export, and report an evidence gap when neither can express or fully return the range. - Add the continuous-period sum pattern: lock source, sum field, date field, complete range, granularity and filters before conclusions, reconcile mutually exclusive bucket sums against the same-scope full-range total, and derive trends from unrounded bucket values. Documentation only; no unit test is required. Co-authored-by: TRAE CLI <noreply@bytedance.com> Co-authored-by: TRAE CLI <traecli@bytedance.com>
1bfd7d1 to
c7d9ecc
Compare
| > - **范围型关键字**（`CurrentWeek`、`LastWeek`、`CurrentMonth`、`LastMonth`、`TheLastWeek`、`TheNextWeek`、`TheLastMonth`、`TheNextMonth`）仅支持 `is` 运算符。 | ||
| > - **关键字大小写敏感**：`ExactDate`、`Today`、`CurrentWeek` 等首字母大写，写错大小写会导致校验失败。 | ||
| > | ||
| > **本地日历范围边界**：`+data-query` 的 datetime 范围只有严格的 `isGreater` / `isLess`，不得把下界向前移动一天来模拟“包含起始日”，否则会把前一天的记录带入总体。对于可按日期重组的指标，可以移除有歧义的范围 filter，改为按日期字段作为 dimension、返回组成该指标且可合并的 measures，并显式设置 `pagination.limit=5000`；只有结果少于 5000 行时，才在本地按返回的日期值筛选并汇总到 `[start, next_start)`。结果达到上限或指标不能从日期维度结果准确重组时，返回统一 SOP 改走可完整导出的 NDJSON 路径，否则报告证据缺口。 |
There was a problem hiding this comment.
这里有问题，目前不支持 isGreaterEqual，所以为了实现包含起始日必须下界前移 1ms，这里不允许前移一天有问题
| ["业务日期", "==", "ExactDate(2026-08-07)"], // 具体一天：按 Base 时区匹配 2026-08-07 当天 | ||
| ["发生时间", ">", "ExactDate(2024-01-31 23:59:59.999)"], // 日期不支持 >=；用 > 前一天最后一毫秒表达含当天的下界 | ||
| ["发生时间", "<", "ExactDate(2024-03-01 00:00:00)"] // 2024 年 2 月范围上界：小于 3 月 1 日零点 | ||
| ["发生时间", "<", "ExactDate(2024-03-01)"] // 范围上界：严格小于 3 月 1 日零点，得到 2 月区间的开区间上界；含首日的下界不要用 > 前一天最后一毫秒，改用下文半开区间 [start, next_start) 口径 |
There was a problem hiding this comment.
这个设计不能删除，目前不支持 >= 所以必须用 -1ms 来模拟
| } | ||
| ``` | ||
|
|
||
| `+record-list` 的 `datetime` 范围条件只使用严格的 `>` / `<`，不要套用通用 tuple 中的 `>=` / `<=`。`ExactDate(...)` 按 Base 时区的日期边界解释，也不能通过把下界前移一天来模拟“包含首日”。需要精确的本地日历半开区间 `[start, next_start)` 时，指标可按日期重组则使用下文 `+data-query` 的日期维度恢复路径；否则按本 SOP 完整导出 NDJSON，再用序列化值中的本地日期筛选。 |
|
|
||
| 例如，`2026-03-20T23:30:00.000-05:00` 与 `2026-03-21T12:30:00.000+08:00` 表示同一时刻；前者若是来源 Base 的值，本地日报归入 3 月 20 日，而时长或排序计算应把它解析为绝对时刻。只构造任务实际需要的日期表示，并在分析引擎中使用具备 datetime 功能的列。 | ||
|
|
||
| 连续日历区间分桶时，从用户请求的范围和粒度生成相邻、不重叠的半开区间 `[start, next_start)`；上界使用下一周期首日，不能把本周期最后一天复用为下一周期起点。 |
There was a problem hiding this comment.
冗余了，一个指令没有必要重复那么多次。另外，前一天最后 1ms 的设计被破坏了，不能改动这个
|
|
||
| ## 常见分析模式 | ||
|
|
||
| ### 连续分期求和 |
|
|
||
| - 对按连续日历周期统计的 `sum`，在第一次用于支撑结论的计算前锁定数据源、求和字段、日期字段、完整范围、粒度和过滤条件；主计算、校验和最终答案复用这些口径。 | ||
| - 在把结果视为完整前，确认 `records_count` / `has_more` 或 Cloud 聚合覆盖目标范围，抽查范围边界记录，并将互斥、完整的分桶值之和与同口径全区间结果对账；不一致时先修正范围或计算。 | ||
| - 当用户要求覆盖完整周期的连续分期结果时，最终交付逐桶结果和同口径全区间合计；逐桶值、合计和趋势直接复用同一份已校验的结构化计算结果，提交前逐项核对。用户要求走势时，基于未舍入的分桶值给出相邻周期变化和首尾比较。 |
PR Quality SummaryCI did not complete successfully. Use the failed check links below to decide whether this PR needs a code change or a rerun. CI status
|
4 participants
Summary
Narrow this documentation change to reusable correctness gaps in continuous calendar-period sums. The shipped guidance stays operation-scoped: it does not introduce business-specific field priorities or defaults for rate calculations.
The top-level
skills/lark-base/SKILL.mdremains unchanged. Calendar-sum workflow guidance lives in the conditional Record query/analysis SOP, while+data-query-specific datetime behavior stays in its DSL reference.Changes
[start, next_start)and the actual+record-listlower-bound capability.+data-querystrict datetime filters cannot express an inclusive local-calendar start exactly, recover through a complete date-dimension result below the 5000-row ceiling or route to complete NDJSON.Scope
Evaluation attribution
The evaluated quarterly sales runs showed that exact period values depended on correct local-calendar membership and complete aggregation input. The remaining reusable delivery gap was omission of the same-scope full-year total. This PR addresses only those two causes; it does not encode expected values or business semantics.
Validation
node scripts/skill-format-check/index.jsQUALITY_GATE_CHANGED_FROM=b6d04738e5933b9f9668acfc7b35b12e631923ac make quality-gategit diff --check b6d04738e5933b9f9668acfc7b35b12e631923ac...HEADRelated Issues