fix: normalize mail triage filters - #2068
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:
📝 WalkthroughWalkthroughChangesMail triage now accepts JSON, token, and Mail triage filter flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Runtime
participant MailTriage
participant FilterBuilder
participant PaginationHint
Runtime->>MailTriage: provide filter and runtime flags
MailTriage->>FilterBuilder: buildTriageFilter(runtime)
FilterBuilder-->>MailTriage: return effective filter
MailTriage->>PaginationHint: appendTriagePaginationFilterFlags(runtime)
PaginationHint-->>Runtime: return next-page hint
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
shortcuts/mail/mail_triage.go (2)
534-549: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCase handling is inconsistent between bare tokens and
key=value.Bare tokens are lowercased (
strings.ToLower(raw)), but the kv key is matched case-sensitively, soIS_UNREADworks whileIS_UNREAD=trueis rejected as an unknown key. Lowercasekeyfor symmetry.♻️ Proposed tweak
key, value, _ := strings.Cut(raw, "=") - key = strings.TrimSpace(key) + key = strings.ToLower(strings.TrimSpace(key)) value = strings.TrimSpace(value)🤖 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/mail/mail_triage.go` around lines 534 - 549, Normalize the key extracted in the key=value path to lowercase before the switch in the triage filter parser, matching the existing strings.ToLower handling for bare tokens. Update the key normalization near strings.Cut so case variants such as IS_UNREAD=true resolve through the same cases as lowercase keys.
447-457: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
dec.Decode(&extra)conflates malformed trailing data with "multiple JSON values".Any non-EOF error (including a syntax error after the object) is reported as multiple JSON values. Minor, but distinguishing
io.EOFfrom a decode error yields a truer message.🤖 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/mail/mail_triage.go` around lines 447 - 457, Update parseTriageFilterJSON’s second dec.Decode(&extra) check to distinguish io.EOF from other decode errors: keep EOF as the valid end-of-input case, but report non-EOF decode errors as invalid JSON trailing data rather than multiple JSON values; reserve the multiple-values message for successfully decoded additional JSON.
🤖 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/mail/mail_triage_test.go`:
- Around line 281-303: Update TestParseTriageFilterRejectsInvalidShorthands to
assert each error’s typed validation metadata via errs.ProblemOf
category/subtype, and use errors.As to verify *errs.ValidationError has Param
set to "--filter". Add a malformed-JSON case and assert its underlying decode
error remains unwrap-able, while retaining the existing message checks.
In `@shortcuts/mail/mail_triage.go`:
- Line 154: Update the pagination hint construction in the mail triage flow to
re-emit all filter flags merged by buildTriageFilter, including --folder,
--folder-id, --is-unread, and --is-read, in addition to --filter. Ensure the
suggested next-page command preserves the active query filters and page-token
semantics.
- Around line 503-518: Make read-status conflict validation deterministic by
processing is_unread before is_read outside the nondeterministic fields map
iteration in the triage filter parsing flow. Reuse the existing bool
unmarshalling and mergeTriageReadStatus logic, and ensure conflicting input
consistently reports is_unread as the offending source.
- Around line 545-583: Update the key=value parsing branch around the triage
filter parser so embedded key=value pairs such as “folder=INBOX,is_unread=true”
are detected and rejected with the existing typed --filter validation error
instead of being stored as part of a scalar value. Preserve valid scalar and
boolean parsing, and ensure unsupported input shapes cannot be silently coerced
or ignored.
---
Nitpick comments:
In `@shortcuts/mail/mail_triage.go`:
- Around line 534-549: Normalize the key extracted in the key=value path to
lowercase before the switch in the triage filter parser, matching the existing
strings.ToLower handling for bare tokens. Update the key normalization near
strings.Cut so case variants such as IS_UNREAD=true resolve through the same
cases as lowercase keys.
- Around line 447-457: Update parseTriageFilterJSON’s second dec.Decode(&extra)
check to distinguish io.EOF from other decode errors: keep EOF as the valid
end-of-input case, but report non-EOF decode errors as invalid JSON trailing
data rather than multiple JSON values; reserve the multiple-values message for
successfully decoded additional JSON.
🪄 Autofix (Beta)
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: 15f942cc-5539-442c-bf8a-ba8e131f76a3
📒 Files selected for processing (2)
shortcuts/mail/mail_triage.goshortcuts/mail/mail_triage_test.go
Change-Type: ci-fix
|
🤖 AI Review | CR 汇总 | 可合入前建议修复(1 个 P2) 增量审查:已读取现有评论并跳过 CodeRabbit 已提出且后续确认解决的问题。本次新增 1 个 P2 正确性问题: 本地仓库 clone 因 GitHub 443 连接超时未能完成,审查基于 |
f3839d2 to
2a82a42
Compare
2a82a42 to
6412493
Compare
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@db9dcba732e0f14a6d1eeaa0f0bde9022ed610f1🧩 Skill updatenpx skills add yangr-happy/cli#feat/12cf2e5 -y -g |
Document the new mail triage filter forms and standalone folder/unread flags so local skill guidance matches CLI help and schema output. Co-authored-by: TRAE CLI <noreply@bytedance.com>
0b7950f to
b119876
Compare
Co-authored-by: TRAE CLI <noreply@bytedance.com>
Reject is_unread=false instead of silently treating it as no read-status filter, and cover the JSON and key=value forms in read-status validation tests. Co-authored-by: TRAE CLI <noreply@bytedance.com>
Summary
Tests
Summary by CodeRabbit
mail +triage --filterparsing to handle whitespace/empty input, JSON, standaloneis_unread/is_readtokens, andkey=valueforms.--filtertext.is_read=false→ unread;is_unread=falseignored;is_read=truerejected with clearer schema guidance and suggestions.--print-filter-schemaexamples to match supported inputs, normalization, and alias behavior.