Skip to content

fix(incident): clarify comment length cap unit and report actual count - #166

Merged
ysyneu merged 1 commit into
mainfrom
fix/incident-comment-char-cap
Aug 27, 2026
Merged

fix(incident): clarify comment length cap unit and report actual count#166
ysyneu merged 1 commit into
mainfrom
fix/incident-comment-char-cap

Conversation

@ysyneu

Copy link
Copy Markdown
Contributor

What

flashduty incident comment rejects comment text longer than 1024 characters. Two usability gaps around that cap:

  1. Ambiguous unit in help text. The help said "at most 1024 characters" without saying how a character is counted. A script measuring with wc -c (bytes) over-trims multibyte text (e.g. Chinese) that is actually compliant.
  2. Over-cap error hid the actual length. The error said only --comment-file content must be at most 1024 characters, leaving the caller to guess how much to trim.

Changes

  • Help text now states the limit counts characters (Unicode runes), not bytes — multibyte text is not penalized — and to measure with wc -m, not wc -c.
  • The over-cap error now reports actual vs limit, e.g. --comment-file content is 1025 characters, limit is 1024. It still fails before any API call is made.

The counting itself was already rune-based (len([]rune(...))), matching the server-side check; the cap value is unchanged. Only the message and help text changed.

Tests

  • Extended the over-cap test to assert the error contains both the actual count and the limit.
  • Extended the help-text test to assert the wc -m guidance is present.
  • The existing test that a 1024-rune multibyte comment is accepted still passes.
  • make fmt, make lint, make test (go test -race ./...), make check-cards, make build all pass.

The 'incident comment' help said 'at most 1024 characters' without stating
the unit, so scripts measuring with wc -c (bytes) over-trim compliant
multibyte text. State that the limit counts Unicode characters (runes),
not bytes, and point at wc -m.
The over-cap error also said only 'must be at most 1024 characters'. It
now reports the actual character count alongside the limit (e.g.
'content is 1025 characters, limit is 1024') so the caller knows how
much to trim. The counting was already rune-based and matches the
server-side check; only the message changed.
@ysyneu
ysyneu changed the base branch from feat/ai-sre to mainAugust 27, 2026 01:55
@ysyneu
ysyneu merged commit 22a7f15 into mainAug 27, 2026
6 of 7 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@ysyneu