feat(base): add typed NDJSON workflows for professional data analysis - #2196
Merged
Conversation
|
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:
📝 WalkthroughWalkthroughThis PR adds typed NDJSON export for Base record list, search, and get commands. It adds manifests, pagination, validation, artifact handling, JQ support, CLI routing, tests, and updated Base analysis guidance. ChangesRecord export pipeline
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant CLI
participant RecordShortcut
participant RecordExporter
participant RecordAPI
participant ArtifactFiles
CLI->>RecordShortcut: select NDJSON format and export options
RecordShortcut->>RecordExporter: validate and start export
RecordExporter->>RecordAPI: request paginated records
RecordAPI-->>RecordExporter: return record matrix pages
RecordExporter->>ArtifactFiles: write NDJSON and manifest
ArtifactFiles-->>CLI: return export metadata
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
github-actions
Bot
added
domain/base
zgz2048
force-pushed
the
codex/base-record-ndjson
branch
from
August 5, 2026 12:16
0c14a6d to
99d6a7d
Compare
zgz2048
marked this pull request as ready for review
August 7, 2026 09:39
…djson # Conflicts: # skills/lark-base/SKILL.md
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@ea6888f6fa49ce700c68655cb7edf62cabfef990🧩 Skill updatenpx skills add zgz2048/cli#codex/base-record-ndjson -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (5)
shortcuts/base/recordexport/dataset_test.go (1)
69-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the comma-ok form for these type assertions.
If a regression changes the cell shape, these assertions panic instead of producing a readable test failure. Use the comma-ok form and call
t.Fatalfwith the actual value.Also applies to: 150-157
🤖 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 `@shortcuts/base/recordexport/dataset_test.go` at line 69, Update the type assertions in the dataset test, including the assertion around first[7] and the additional assertions at the referenced locations, to use comma-ok checks. When an assertion fails, call t.Fatalf with the actual cell value and stop the test; otherwise continue using the successfully asserted map value.shortcuts/base/record_export.go (1)
193-276: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared pagination loop.
executeRecordListNDJSONandexecuteRecordSearchNDJSONcontain the same loop. Only the request call differs. The row-count guard, the accumulate step, theremaining/currentOffsetarithmetic, and the termination condition are duplicated. Extract a helper that takes afunc(offset, limit int) (map[string]any, error)fetcher, so a future fix to the pagination arithmetic applies to both commands.🤖 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 `@shortcuts/base/record_export.go` around lines 193 - 276, Extract the duplicated pagination logic from executeRecordListNDJSON and executeRecordSearchNDJSON into a shared helper that accepts a func(offset, limit int) (map[string]any, error) fetcher. Keep page parsing, row-count validation, accumulation, offset/remaining updates, and termination behavior in the helper; have each command provide only its request-specific fetcher and retain its existing finalization flow.shortcuts/base/recordexport/manifest.go (2)
336-361: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
bestExamplemarshals every cell of every column.The function calls
json.Marshalonce per record per column to find the shortest non-empty value. For the 2000-row NDJSON limit and a wide table, this runs tens of thousands of extra marshals after the rows are already written. Stop the scan once a value is small enough, for example once the encoded length is at or below a threshold, or cap the scan at the first N records.The name is also misleading. The function selects the shortest example, not the most representative one. Consider
shortestExample.🤖 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 `@shortcuts/base/recordexport/manifest.go` around lines 336 - 361, Limit the work performed by bestExample while preserving selection of the shortest non-empty value: stop scanning once an encoded value meets a small-size threshold or after a bounded number of records, using a clearly defined constant. Rename bestExample and its callers to shortestExample to accurately describe the behavior.
65-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueColumn order is lost in the manifest.
Columnsis amap[string]ColumnManifest, so the manifest does not preserve the dataset column order. A consumer that builds a table or a DataFrame from the manifest cannot reproduce the original field order. Add an orderedcolumn_order []stringfield, or changeColumnsto an ordered slice.Also applies to: 121-135
🤖 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 `@shortcuts/base/recordexport/manifest.go` at line 65, Add an ordered column sequence to the manifest model alongside Columns, such as a column_order []string JSON field, and populate it wherever the manifest is constructed or serialized (including the code around lines 121-135). Preserve the existing column definitions while ensuring consumers can reconstruct the original dataset field order.shortcuts/base/recordexport/dataset.go (1)
274-283: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider detecting
revdrift across pages.
AppendPagecompares the timezone and the source columns, but notRev.recordExportAccumulatorkeeps the first page'sRev, so the manifest labels the whole dataset with a revision that later rows no longer match.TestRecordListNDJSONSerializesPagesAbove200inshortcuts/base/record_export_test.goasserts this behavior withrev100 and 101, so it looks intentional. If it is intentional, record the drift in the manifest or in a stderr warning, because row-level data changed between pages while the schema did not.🤖 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 `@shortcuts/base/recordexport/dataset.go` around lines 274 - 283, Update Dataset.AppendPage and the manifest accumulation flow to detect when page.Dataset.Rev differs from the dataset’s initial Rev, while preserving the existing successful append behavior. Record the revision drift in the manifest or emit a stderr warning so consumers are informed that row data spans multiple revisions; retain the existing first-Rev behavior unless the surrounding contract requires otherwise.
🤖 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 `@shortcuts/base/base_execute_test.go`:
- Around line 2821-2829: The BaseRecordBatchUpdate test should assert the
complete ignored_fields structure rather than only checking that stdout contains
“ignored_fields” and “Formula”. Validate the ignored field’s id, name, and
reason values, matching the structured contract asserted by the corresponding
markdown test.
In `@shortcuts/base/record_export_test.go`:
- Around line 234-241: Update the collision test around runShortcut and the
jq-records table test around lines 403-414 to validate typed errors with the
existing assertInvalidArgumentValidation pattern. Assert the expected category,
subtype, and parameter, and verify cause preservation through errs.ProblemOf
instead of relying only on problem.Hint or err.Error() substring checks.
In `@shortcuts/base/record_export.go`:
- Around line 325-346: Update the error path after saveRecordManifest in the
record export flow to remove the record file created by saveRecordNDJSON before
returning the manifest-save error. Preserve the original manifest error and
ensure cleanup targets paths.recordRelative through the existing file I/O
abstraction.
In `@shortcuts/base/recordexport/dataset_test.go`:
- Around line 180-193: Update the error-path tests around
TestDatasetAppendRejectsSchemaChange and the related tests near the referenced
range to assert concrete error types with errors.As: expect *SchemaChangedError
for schema changes and *MatrixError for matrix failures. Verify the typed error
metadata, including the changed field where applicable, and preserve/assert the
underlying cause instead of checking only err != nil or message text.
In `@shortcuts/base/recordexport/dataset.go`:
- Around line 399-412: Update the ignored_fields parsing block in the record
export flow so missing id, name, or reason keys default to empty strings without
returning a MatrixError. Retain the type validation for present values,
returning the existing error when any supplied value is not a string, and
continue processing advisory warning objects without aborting valid exports.
- Around line 171-188: Validate column names while building exportColumns in the
surrounding dataset construction flow, rejecting any duplicate non-system names
with a MatrixError before returning the page; preserve the existing
RecordIDColumnName handling so the system join key continues to take precedence
over a same-named source field.
In `@skills/lark-base/references/lark-base-cell-value.md`:
- Around line 64-72: Align the datetime serialization contract across the
serializer, the artifact test, and the documentation: choose one RFC3339 output
format regarding milliseconds and apply it consistently. Update the relevant
record-export serializer and its test, then revise the datetime read-value
example in the cell-value documentation to match.
- Around line 51-60: Update the select-field examples in the complete example
and the record upsert example to use option-name arrays consistently with the
documented select write/read contract, replacing scalar "状态" values while
preserving the examples’ intended data.
In `@skills/lark-base/references/lark-base-data-analysis-sop.md`:
- Around line 168-172: 在“单表简单筛选与统计：jq”段落中，将本地分析说明里的 `js -s` 更正为 `jq -s`，保持与前文
`jq -s` 工作流及 `--jq-records` 等价命令一致。
- Around line 134-136: Update the physical-type table entries around the rows
containing inline code at lines 134–136 and 139 so every literal pipe is escaped
as \| (or replaced with an equivalent representation that contains no raw table
delimiter). Preserve the documented type and example values while keeping the
Markdown table correctly aligned.
In `@skills/lark-base/SKILL.md`:
- Around line 34-36: Clarify the workflow guidance around the Base command
section so lark-drive is used only for file import/export, while record
list/search/get operations that produce local NDJSON analysis artifacts remain
in this Base workflow. Update the related routing statements, including the
references near the high-frequency data-analysis guidance and other
Base-to-local export instructions, without changing the copy or table-copy
paths.
In `@tests/cli_e2e/base/base_record_list_dryrun_test.go`:
- Around line 118-169: Add live E2E coverage alongside the dry-run tests, using
the existing base-record test helpers and bot credential configuration to create
a temporary base/table and records, run record-list, record-search, and
record-get with NDJSON output, and verify real records plus NDJSON and manifest
publication. Make the flow self-contained with deferred cleanup of all created
resources and generated artifacts, while preserving the existing dry-run
assertions.
- Around line 147-153: Update the test assertions in the record-list dry-run
test to verify that data.output equals the requested search.ndjson destination.
Keep the existing export-format and requested-limit assertions unchanged, and
assert the output path directly so destination handling is covered.
---
Nitpick comments:
In `@shortcuts/base/record_export.go`:
- Around line 193-276: Extract the duplicated pagination logic from
executeRecordListNDJSON and executeRecordSearchNDJSON into a shared helper that
accepts a func(offset, limit int) (map[string]any, error) fetcher. Keep page
parsing, row-count validation, accumulation, offset/remaining updates, and
termination behavior in the helper; have each command provide only its
request-specific fetcher and retain its existing finalization flow.
In `@shortcuts/base/recordexport/dataset_test.go`:
- Line 69: Update the type assertions in the dataset test, including the
assertion around first[7] and the additional assertions at the referenced
locations, to use comma-ok checks. When an assertion fails, call t.Fatalf with
the actual cell value and stop the test; otherwise continue using the
successfully asserted map value.
In `@shortcuts/base/recordexport/dataset.go`:
- Around line 274-283: Update Dataset.AppendPage and the manifest accumulation
flow to detect when page.Dataset.Rev differs from the dataset’s initial Rev,
while preserving the existing successful append behavior. Record the revision
drift in the manifest or emit a stderr warning so consumers are informed that
row data spans multiple revisions; retain the existing first-Rev behavior unless
the surrounding contract requires otherwise.
In `@shortcuts/base/recordexport/manifest.go`:
- Around line 336-361: Limit the work performed by bestExample while preserving
selection of the shortest non-empty value: stop scanning once an encoded value
meets a small-size threshold or after a bounded number of records, using a
clearly defined constant. Rename bestExample and its callers to shortestExample
to accurately describe the behavior.
- Line 65: Add an ordered column sequence to the manifest model alongside
Columns, such as a column_order []string JSON field, and populate it wherever
the manifest is constructed or serialized (including the code around lines
121-135). Preserve the existing column definitions while ensuring consumers can
reconstruct the original dataset field order.
🪄 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: 3340746b-2a55-4fc3-8b73-d5e3e945339f
📒 Files selected for processing (31)
shortcuts/base/base_execute_test.goshortcuts/base/base_shortcuts_test.goshortcuts/base/record_export.goshortcuts/base/record_export_test.goshortcuts/base/record_get.goshortcuts/base/record_json_shorthand_test.goshortcuts/base/record_list.goshortcuts/base/record_markdown.goshortcuts/base/record_markdown_test.goshortcuts/base/record_ops.goshortcuts/base/record_query.goshortcuts/base/record_search.goshortcuts/base/recordexport/dataset.goshortcuts/base/recordexport/dataset_test.goshortcuts/base/recordexport/errors.goshortcuts/base/recordexport/manifest.goshortcuts/base/recordexport/ndjson.goshortcuts/common/runner.goshortcuts/common/types.goskills/lark-base/SKILL.mdskills/lark-base/references/lark-base-cell-value.mdskills/lark-base/references/lark-base-data-analysis-cloud.mdskills/lark-base/references/lark-base-data-analysis-pandas.mdskills/lark-base/references/lark-base-data-analysis-python-stdlib.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.mdskills/lark-base/references/lark-base-field-json.mdskills/lark-base/references/lark-base-record-upsert.mdtests/cli_e2e/base/base_record_list_dryrun_test.gotests/cli_e2e/base/base_skill_contract_test.go
💤 Files with no reviewable changes (1)
- tests/cli_e2e/base/base_skill_contract_test.go
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/lark-base/references/lark-base-data-analysis-sop.md (1)
39-39: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the February lower-bound example.
The
>condition withExactDate(2024-01-31 23:59:59)includes records from January 31 between23:59:59.000and midnight when timestamps contain milliseconds. Use February 1 at midnight as the lower bound.Proposed fix
- ["发生时间", ">", "ExactDate(2024-01-31 23:59:59)"], // 2024 年 2 月范围下界：闰年 2 月包含 29 日 + ["发生时间", ">=", "ExactDate(2024-02-01 00:00:00)"], // 2024 年 2 月范围下界：闰年 2 月包含 29 日🤖 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 `@skills/lark-base/references/lark-base-data-analysis-sop.md` at line 39, Update the February date-range example in the SOP to use February 1, 2024 at midnight as the lower bound, replacing the January 31 23:59:59 ExactDate value while preserving the existing `>` condition.
🤖 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.
Outside diff comments:
In `@skills/lark-base/references/lark-base-data-analysis-sop.md`:
- Line 39: Update the February date-range example in the SOP to use February 1,
2024 at midnight as the lower bound, replacing the January 31 23:59:59 ExactDate
value while preserving the existing `>` condition.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 52449e61-76f6-44c0-951e-4ef5aabbb1c3
📒 Files selected for processing (10)
shortcuts/base/record_export.goshortcuts/base/record_export_test.goshortcuts/base/record_list.goshortcuts/base/record_query.goshortcuts/base/record_search.goskills/lark-base/SKILL.mdskills/lark-base/references/lark-base-data-analysis-cloud.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 (8)
- shortcuts/base/record_list.go
- shortcuts/base/record_export.go
- shortcuts/base/record_search.go
- shortcuts/base/record_query.go
- skills/lark-base/references/lark-base-data-query.md
- skills/lark-base/SKILL.md
- skills/lark-base/references/lark-base-data-analysis-cloud.md
- shortcuts/base/record_export_test.go
…djson # Conflicts: # skills/lark-base/SKILL.md
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/lark-base/references/lark-base-data-analysis-sop.md (1)
115-116: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not treat the first-page
revas a snapshot-consistency guarantee.The exporter sends independent offset requests and stores only the first page's
rev. Comparing it with the latest tablerevcan detect some changes, but cannot prove that all pages used the same revision. Describe this check as best effort. If stronger validation is required, retain per-page revisions and flag mismatches.🤖 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 `@skills/lark-base/references/lark-base-data-analysis-sop.md` around lines 115 - 116, 修改该 SOP 中关于 manifest.rev 与最新 table rev 比较的表述：明确首个响应页的 rev 只能用于尽力检测版本变化，不能保证所有分页请求使用同一快照。若需要更强校验，说明应保留每页 rev 并标记不一致。
🤖 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`:
- Line 48: 修正“全局结论”的完整性判定：不要仅凭 has_more=false 认定导出覆盖全表；在使用导出结果前，确认查询从预期的起始
offset 开始且未截断，或明确将完整性限定为请求窗口。同步更新该 SOP 中对 has_more=false 的表述，保留 has_more=true
时继续收敛谓词或选择 Cloud 路径的逻辑。
- Around line 39-40: Update the date-range example around the “发生时间” filters to
use only the documented ExactDate(YYYY-MM-DD HH:mm) format, avoiding seconds and
millisecond precision while preserving the intended February lower and upper
boundaries.
---
Outside diff comments:
In `@skills/lark-base/references/lark-base-data-analysis-sop.md`:
- Around line 115-116: 修改该 SOP 中关于 manifest.rev 与最新 table rev 比较的表述：明确首个响应页的 rev
只能用于尽力检测版本变化，不能保证所有分页请求使用同一快照。若需要更强校验，说明应保留每页 rev 并标记不一致。
🪄 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: b047e70a-d122-41c9-9869-4b3a24f7fdb6
📒 Files selected for processing (1)
skills/lark-base/references/lark-base-data-analysis-sop.md
kongenpei
approved these changes
Aug 11, 2026
Merged
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a typed NDJSON artifact workflow for Base record reads so agents can perform professional local analytics—joins, window and calendar calculations, multi-value expansion, attribution, and deeper insight generation—without loading hundreds of raw records into the model context.
The existing Markdown and raw JSON paths remain compatible. The new path complements Base cloud aggregation: agents use local analysis when a complete task dataset can be kept within 2,000 records per table, and fall back to the Cloud SOP when it cannot.
Why
The current Base record response is a compact matrix (
fields,field_id_list,field_type_list,record_id_list, anddata[][]). It is efficient on the wire, but requires every downstream agent to reconstruct rows and types before using jq, Python, or a dataframe engine. Returning the matrix inline also consumes model tokens and makes repeated analysis expensive.This feature moves the deterministic transformation into
lark-clionce:Changes
Typed Base record artifacts
--format ndjsontobase +record-list,+record-search, and+record-get.--output <path>.ndjson; an output path implies NDJSON unless a conflicting format was explicitly provided.<path>.manifest.json, while stdout returns the manifest rather than record bodies.--minimal-stdoutfor repeated workflows that only need artifact paths,records_count, andhas_more.--overwritewith actionable collision errors.--jquseful for NDJSON mode by applying it to the JSON manifest on stdout.Complete local reads up to 2,000 records
--limitvalues up to 2,000 while keeping inline Markdown/JSON reads capped at 200.Stable typed row model and manifest
record_idas the non-null join key; it wins over a user field with the same name.field_idandfield_typein manifest column metadata.[]and checkbox empty cells tofalse.revwhen available; it is omitted for older server versions that do not return it.Agent analysis routing and professional Base semantics
records_count, with predicate/projection pushdown for large tables before choosing a path.+data-queryDSL out of context unless the Cloud path actually selects it, reducing prompt size and avoiding mixed execution plans.SKILL.md; behavior is covered at the command/export boundary instead.User and agent impact
Test Plan
make buildmake unit-test(full-raceunit suite acrosscmd,internal,shortcuts, andextension)go test ./shortcuts/base/... ./shortcuts/common/... ./tests/cli_e2e/base/...go vet ./...gofmt -l .returned no filesgo mod tidyproduced nogo.mod/go.sumchangesgolangci-lint v2.1.6 run --new-from-rev=upstream/mainreturned0 issues+table-list: all 50 returned tables contained numericrecords_countandrev.+base-block-list: all 94 returned table blocks contained numericrecords_countandrev.+record-list, stdout manifest, and file manifest returned the same table revision (998in the test Base).Related Issues
Summary by CodeRabbit