Skip to content

fix(base): clarify table-id tbl prefix requirement#270

Merged
kongenpei merged 1 commit intomainfrom
fix/base-table-id-tbl-prefix
Apr 7, 2026
Merged

fix(base): clarify table-id tbl prefix requirement#270
kongenpei merged 1 commit intomainfrom
fix/base-table-id-tbl-prefix

Conversation

@kongenpei
Copy link
Copy Markdown
Collaborator

@kongenpei kongenpei commented Apr 5, 2026

Summary

Clarifies --table-id usage for Base table operations by explicitly stating that when an ID is provided, it must start with tbl, while table names are still supported.

Changes

  • Updated shared Base shortcut flag description in shortcuts/base/base_command_common.go to: table ID (must start with tbl if ID) or name.
  • Updated skills/lark-base/references/lark-base-table-get.md parameter docs to emphasize the tbl prefix rule for IDs.
  • Added a pitfall note in skills/lark-base/references/lark-base-table-get.md to guide users to ask for the target table or run +table-list when input is not a valid tbl_* ID.

Test Plan

  • Unit tests pass
  • Manual local verification confirms the lark xxx command works as expected

Related Issues

  • None

@github-actions github-actions bot added domain/base PR touches the base domain size/M Single-domain feat or fix with limited business impact labels Apr 5, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 5, 2026

📝 Walkthrough

Walkthrough

Two files updated to clarify that table IDs must start with the tbl prefix: a flag description in Go source code and documentation with validation guidance for CLI users.

Changes

Cohort / File(s) Summary
Table ID Prefix Clarification
shortcuts/base/base_command_common.go, skills/lark-base/references/lark-base-table-get.md
Updated flag description and documentation to specify that table IDs must start with tbl prefix; added validation warning to guide users when providing table identifiers.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A tiny hop to clarity we go,
Where tbl prefixes help the table IDs flow,
The docs now gleam with warnings bright,
No more confusion in the CLI night! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: clarifying that table IDs must start with 'tbl' prefix, which matches the primary focus of both file modifications.
Description check ✅ Passed The pull request description follows the template with all required sections completed: summary clearly explains the motivation, changes are well-documented, test plan includes both unit tests and manual verification checkmarks, and related issues are addressed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/base-table-id-tbl-prefix

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 5, 2026

Greptile Summary

This PR clarifies the --table-id flag description across base_command_common.go (shared for all base subcommands) and adds a pitfall note in lark-base-table-get.md explaining that when passing an ID, it must start with tbl. The shared Go flag change propagates to all commands using tableRefFlag, while only lark-base-table-get.md was updated in the docs — other reference files (e.g., lark-base-field-list.md, lark-base-record-list.md) still describe --table-id as "表 ID 或表名" without the tbl prefix requirement.

Confidence Score: 5/5

Safe to merge — changes are purely descriptive/documentation with no logic changes.

Both changed files contain only documentation string updates; no logic, API, or schema is affected. The only remaining finding is a P2 suggestion to extend the same doc update to sibling reference files.

No files require special attention. The sibling .md files are a P2 completeness suggestion, not a blocker.

Important Files Changed

Filename Overview
shortcuts/base/base_command_common.go Updated tableRefFlag description to clarify the tbl prefix requirement for table IDs; change is minimal and correct.
skills/lark-base/references/lark-base-table-get.md Added tbl prefix requirement to the parameter table and a new pitfall bullet; documentation is accurate and helpful.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User provides --table-id] --> B{Is it an ID or a name?}
    B -->|Name| C[Resolve by name match]
    B -->|ID| D{Starts with 'tbl'?}
    D -->|Yes| E[Use directly as table_id in API call]
    D -->|No| F[⚠️ Ask user which table, or run +table-list to confirm]
    C --> G[Proceed with API call]
    E --> G
Loading

Comments Outside Diff (1)

  1. skills/lark-base/references/lark-base-field-list.md

    P2 tbl prefix note missing in sibling docs

    The shared tableRefFlag description was updated in Go (affecting all commands), and lark-base-table-get.md received matching doc updates, but the other reference files still describe --table-id as just "表 ID 或表名" without the tbl prefix requirement. Files like lark-base-field-list.md and lark-base-record-list.md have the same pattern, and users relying on those docs may still be confused. Consider applying the same clarification (parameter row + pitfall bullet) to the other affected .md files in skills/lark-base/references/.

Reviews (1): Last reviewed commit: "fix(base): clarify table-id tbl prefix r..." | Re-trigger Greptile

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
skills/lark-base/references/lark-base-table-get.md (1)

39-39: Clarify the intended audience for this guidance.

The warning states "如果 --table-id 传的是 id,必须是 tbl 开头;不是的话先询问用户具体是哪张表,或先用 +table-list 查表列表再确认" (If --table-id is an id, it must start with tbl; if not, ask the user which table they mean, or use +table-list first).

The phrase "先询问用户" (ask the user) suggests this guidance is for developers or AI assistants helping users, not for end users directly. Consider clarifying:

  • Is this validation performed by the CLI tool automatically, or is it manual guidance?
  • If it's for AI assistants/helpers, should this be in a separate developer guide rather than user-facing reference documentation?
📝 Suggested rewording for clarity

If this is guidance for users encountering errors:

-- ⚠️ 如果 `--table-id` 传的是 `id`,必须是 `tbl` 开头;不是的话先询问用户具体是哪张表,或先用 `+table-list` 查表列表再确认。
+- ⚠️ 如果 `--table-id` 传的是 `id`,必须是 `tbl` 开头的表 ID(例如 `tbl_xxx`)。如果不确定表 ID,可以先用 `+table-list` 查询表列表。

If this is guidance for AI assistants/developers:

-- ⚠️ 如果 `--table-id` 传的是 `id`,必须是 `tbl` 开头;不是的话先询问用户具体是哪张表,或先用 `+table-list` 查表列表再确认。
+- ⚠️ **开发者注意:** 如果 `--table-id` 传的是 `id`,必须是 `tbl` 开头;不是的话先询问用户具体是哪张表,或建议用户先用 `+table-list` 查表列表再确认。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/lark-base/references/lark-base-table-get.md` at line 39, The warning
about `--table-id` requiring IDs to start with `tbl` is ambiguous about the
intended audience; update the text around the `--table-id` guidance in
lark-base-table-get.md to state whether the CLI performs this validation
automatically or whether this is an instruction for AI assistants/developers,
and if it is for assistants, move or duplicate the guidance into
developer-facing docs (or prepend a note like "For assistants/developers: ...").
Mention the specific symbols `--table-id` and `+table-list` in the rewritten
sentence so readers know the exact commands referenced.
shortcuts/base/base_command_common.go (1)

17-17: Client-side validation for tbl prefix would improve user experience, but is not required since the Lark API enforces it.

The flag description correctly states table IDs "must start with tbl if ID"—this is enforced by the Lark API, which returns error code 1254004 ("WrongTableId") for invalid IDs. However, the current implementation relies on API-side validation rather than catching this upfront. While functional, users would receive API error responses instead of clear CLI feedback. Adding client-side validation in resolveTableRef would provide better error messages and fail faster.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/base/base_command_common.go` at line 17, Add client-side validation
in resolveTableRef to detect and reject invalid table IDs before calling the
API: if the provided table-ref (from the "table-id" flag) appears to be an ID
form but does not start with the "tbl" prefix, return a clear CLI error
explaining "table IDs must start with 'tbl'"; implement this check at the start
of resolveTableRef (and/or where the "table-id" flag is parsed) so users get
immediate, friendly feedback instead of an API WrongTableId (1254004) response.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@shortcuts/base/base_command_common.go`:
- Line 17: Add client-side validation in resolveTableRef to detect and reject
invalid table IDs before calling the API: if the provided table-ref (from the
"table-id" flag) appears to be an ID form but does not start with the "tbl"
prefix, return a clear CLI error explaining "table IDs must start with 'tbl'";
implement this check at the start of resolveTableRef (and/or where the
"table-id" flag is parsed) so users get immediate, friendly feedback instead of
an API WrongTableId (1254004) response.

In `@skills/lark-base/references/lark-base-table-get.md`:
- Line 39: The warning about `--table-id` requiring IDs to start with `tbl` is
ambiguous about the intended audience; update the text around the `--table-id`
guidance in lark-base-table-get.md to state whether the CLI performs this
validation automatically or whether this is an instruction for AI
assistants/developers, and if it is for assistants, move or duplicate the
guidance into developer-facing docs (or prepend a note like "For
assistants/developers: ..."). Mention the specific symbols `--table-id` and
`+table-list` in the rewritten sentence so readers know the exact commands
referenced.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a0dbe63c-b491-4f35-92d2-92078f34037d

📥 Commits

Reviewing files that changed from the base of the PR and between 0c77c95 and 1bdc919.

📒 Files selected for processing (2)
  • shortcuts/base/base_command_common.go
  • skills/lark-base/references/lark-base-table-get.md

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 5, 2026

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@1bdc919b4ef82ddf3f7c38458977b7f7bdc0872f

🧩 Skill update

npx skills add larksuite/cli#fix/base-table-id-tbl-prefix -y -g

@kongenpei kongenpei requested a review from zgz2048 April 5, 2026 14:56
Copy link
Copy Markdown
Collaborator

@zgz2048 zgz2048 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kongenpei kongenpei merged commit 1be9a24 into main Apr 7, 2026
17 checks passed
@kongenpei kongenpei deleted the fix/base-table-id-tbl-prefix branch April 7, 2026 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/base PR touches the base domain size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants