Skip to content

feat: display rate limit warnings instead of raw JSON#388

Merged
tiann merged 8 commits intotiann:mainfrom
junmo-kim:feat/rate-limit-event-display
Apr 3, 2026
Merged

feat: display rate limit warnings instead of raw JSON#388
tiann merged 8 commits intotiann:mainfrom
junmo-kim:feat/rate-limit-event-display

Conversation

@junmo-kim
Copy link
Copy Markdown
Contributor

Summary

  • Parse Claude's rate_limit_event JSON in the CLI adapter layer and convert to standardized pipe-delimited text
  • Display human-readable rate limit warnings and limit type (5-hour, 7-day) in the web UI
  • Backward compatible: existing limit-reached format without limitType still parses correctly

Motivation

Claude emits rate_limit_event JSON on stdout when approaching or exceeding usage limits. HAPI currently doesn't recognize this message type, so users see raw JSON in the chat UI — including noisy status: "allowed" events.

Architecture

The CLI adapter (AcpMessageHandler) intercepts the undocumented Claude JSON and converts it to a standardized text format that follows the existing "Claude AI usage limit reached|{ts}" convention. The web layer only parses the standardized text — it has no knowledge of Claude's internal JSON format.

Claude stdout → CLI adapter (rateLimitParser) → pipe-delimited text → Web (reducerEvents)

Changes

  • cli/src/agent/rateLimitParser.ts (new): Parse rate_limit_event JSON → pipe-delimited text
  • cli/src/agent/backends/acp/AcpMessageHandler.ts: Integrate parser in flushText()
  • web/src/chat/types.ts: Add limit-warning event type, extend limit-reached with limitType
  • web/src/chat/reducerEvents.ts: Parse warning/reached text with limitType
  • web/src/chat/presentation.ts: Display warnings with formatResetTime() and formatLimitType()
  • web/src/chat/reconcile.ts: Add event key for limit-warning

Test plan

  • CLI: 10 tests for rateLimitParser (five_hour, seven_day, missing type, rejected, allowed, unknown, wrapped)
  • Web: 21 tests for reducerEvents + presentation
  • All existing tests pass
  • Existing limit-reached format without limitType still works (backward compat test)

Extract the shared seconds-vs-milliseconds normalization logic into
a private `normalizeTimestamp()` helper. No behavior change —
`formatUnixTimestamp()` produces identical output.
Change return type from `number | null` to `AgentEvent | null` so
the caller doesn't need to construct the event object. No behavior
change — the same `limit-reached` event is produced.
Parse undocumented Claude `rate_limit_event` JSON in the CLI adapter
layer (AcpMessageHandler) before it reaches the web.

Converted text format (pipe-delimited):
  - "Claude AI usage limit warning|{ts}|{pct}|{rateLimitType}"
  - "Claude AI usage limit reached|{ts}|{rateLimitType}"

Status handling:
  - `allowed_warning` → warning text with utilization and limit type
  - `rejected` → reached text with limit type
  - `allowed` → silently suppressed (noise)
  - unknown statuses → passed through as-is (forward-compatible)
Parse standardized pipe-delimited text from the CLI adapter into
`limit-warning` and `limit-reached` events, displaying the rate
limit type (5-hour, 7-day) when available.

- `limit-warning`: "⚠️ Usage limit 90% (5-hour) · resets 2:00 PM"
- `limit-reached`: "⏳ Usage limit reached (5-hour) until 4/2/2026"
- Backward compatible: `limit-reached` without limitType still works

The `reached` regex uses `(?:\|([^|]*))?$` to optionally match the
limitType field, maintaining compatibility with the existing format.
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Findings

  • [Major] Rate-limit parsing only runs after the entire ACP text buffer has been merged, so a turn that contains both a rate_limit_event chunk and normal assistant text will still fall back to raw text. flushText() parses the full bufferedText, and parseRateLimitText() only matches when the whole string is valid JSON. Evidence cli/src/agent/backends/acp/AcpMessageHandler.ts:114, cli/src/agent/rateLimitParser.ts:21.
    Suggested fix:
    if (text) {
        const rateLimit = parseRateLimitText(text)
        if (rateLimit) {
            this.flushText()
            if (!rateLimit.suppress) this.onMessage(rateLimit.message)
            return
        }
        this.appendTextChunk(text)
    }

Summary

  • Review mode: initial. One issue found in the CLI adapter path: the new parser only handles buffers that are entirely JSON, so mixed turns can still surface raw rate-limit payloads.

Testing

  • Not run (automation). bun is not installed in this environment, so I validated this from source. Suggested follow-up: add an AcpMessageHandler test where a rate_limit_event chunk is followed by ordinary assistant text in the same prompt.

HAPI Bot

this.onMessage({ type: 'text', text: this.bufferedText });
const text = this.bufferedText;
this.bufferedText = '';
const rateLimitResult = parseRateLimitText(text);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[MAJOR] This only parses the final merged buffer. Because bufferedText accumulates all agent_message_chunk text until flushText(), any turn that contains a rate_limit_event chunk plus ordinary assistant text will reach parseRateLimitText() as a mixed string and fall through to raw JSON. Handle rate-limit chunks before they enter the shared text buffer, for example in the agentMessageChunk branch:

if (text) {
    const rateLimit = parseRateLimitText(text)
    if (rateLimit) {
        this.flushText()
        if (!rateLimit.suppress) this.onMessage(rateLimit.message)
        return
    }
    this.appendTextChunk(text)
}

Remove rate limit detection from flushText() back to plain buffer
flush. The next commit will re-add parsing at the chunk level
(handleUpdate) where it can intercept before buffer merging.

Includes failing tests that demonstrate the mixed-chunk bug:
when a rate_limit_event chunk arrives in the same turn as normal
text, the JSON leaks into the merged buffer.
Move rate limit detection from flushText() to the agentMessageChunk
handler so it fires before the chunk enters the shared text buffer.

Previously, a rate_limit_event chunk arriving in the same turn as
normal text would merge into bufferedText and leak as raw JSON.
Now the chunk is intercepted individually, the existing buffer is
flushed first (preserving prior text), and the converted message
is emitted separately.
@junmo-kim
Copy link
Copy Markdown
Contributor Author

Thanks for the review! Fixed in 4478ad2 (refactor) + 963df71 (fix).

Moved rate limit detection from flushText() to the agentMessageChunk handler, exactly as suggested:

if (text) {
    const rateLimit = parseRateLimitText(text);
    if (rateLimit) {
        this.flushText();
        if (!rateLimit.suppress) this.onMessage(rateLimit.message);
        return;
    }
    this.appendTextChunk(text);
}

Added two regression tests covering:

  1. Mixed turn (normal text + rate_limit_event chunk) — normal text preserved, rate limit converted
  2. allowed status suppressed without polluting the text buffer

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Findings

  • [Major] Suppressed allowed rate-limit chunks still flush the accumulated assistant buffer, so any normal text before and after the noisy event gets split into separate chat messages. Because flushText() runs before rateLimit.suppress is checked, a hidden status update can fragment markdown/code fences mid-response. Evidence cli/src/agent/backends/acp/AcpMessageHandler.ts:163.
    Suggested fix:
    const rateLimit = parseRateLimitText(text)
    if (rateLimit) {
        if (rateLimit.suppress) {
            return
        }
        this.flushText()
        this.onMessage(rateLimit.message)
        return
    }

Summary

  • Review mode: follow-up after new commits. One issue remains in the CLI adapter: suppressed allowed events still force a text flush, which fragments assistant output whenever Claude emits hidden rate-limit noise mid-turn.

Testing

  • Not run (automation). Source review only. Suggested follow-up: add an AcpMessageHandler test with text -> allowed rate_limit_event -> text and assert the buffered text stays contiguous.

HAPI Bot

if (text) {
const rateLimit = parseRateLimitText(text);
if (rateLimit) {
this.flushText();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[MAJOR] allowed events are suppressed, but this still flushes the assistant text buffer before the suppression check. If Claude emits text -> allowed rate_limit_event -> text, the invisible event splits one answer into two agent-text blocks and can break markdown/code fences. Only flush when the parsed event is actually displayed.

const rateLimit = parseRateLimitText(text)
if (rateLimit) {
    if (rateLimit.suppress) {
        return
    }
    this.flushText()
    this.onMessage(rateLimit.message)
    return
}

Only flush the text buffer when the parsed event will actually be
displayed. Suppressed events (e.g. status: 'allowed') now return
immediately without flushing, preventing a text → allowed → text
sequence from splitting one answer into two agent-text blocks.
@junmo-kim
Copy link
Copy Markdown
Contributor Author

Fixed in ec46b1c. Only flush the text buffer when the event will actually be displayed:

const rateLimit = parseRateLimitText(text);
if (rateLimit) {
    if (rateLimit.suppress) {
        return;
    }
    this.flushText();
    this.onMessage(rateLimit.message);
    return;
}

Added regression test: text → allowed → text → flush now produces a single merged text message instead of two split blocks.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Findings

  • [Major] limit-reached reconciliation still keys only on endsAt, so when an existing event is refreshed from the old pipe format (Claude AI usage limit reached|<ts>) to the new typed format (...|<ts>|five_hour), the old block is treated as unchanged and the new (5-hour) / (7-day) label never renders. Evidence web/src/chat/reconcile.ts:87.
    Suggested fix:
    case 'limit-reached':
        return `limit:${event.endsAt}:${event.limitType}`

Summary

  • Review mode: follow-up after new commits. One regression remains in the web reconciliation layer: limit-reached now carries limitType, but block equality still ignores that field, so UI refreshes can keep showing the stale untyped message.

Testing

  • Not run (automation). bun is not installed in this runner, so validation was source review only.

HAPI Bot

case 'limit-reached':
return `limit:${event.endsAt}`
case 'limit-warning':
return `limit-warning:${event.endsAt}:${(event as Record<string, unknown>).utilization}:${(event as Record<string, unknown>).limitType}`
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[MAJOR] The new limit-warning key includes all of its display fields, but limit-reached above still ignores the newly added limitType. If the same message ID is reprocessed from the old format to the new typed format, reconciliation reuses the stale block and the (5-hour) / (7-day) suffix never appears.

case 'limit-reached':
    return `limit:${event.endsAt}:${event.limitType}`

Without this, reprocessing a message from the old format (no
limitType) to the new typed format reuses the stale block and
the (5-hour)/(7-day) suffix never appears.
@junmo-kim
Copy link
Copy Markdown
Contributor Author

Fixed in 8bae337. Added limitType to the limit-reached reconcile key:

case 'limit-reached':
    return `limit:${event.endsAt}:${(event as Record<string, unknown>).limitType}`

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Findings

  • None.

Summary

  • Review mode: follow-up after new commits. No additional diff-attributable issues found on the current head. Residual risk: validation here was source review only because bun is not installed in this runner.

Testing

  • Not run (automation). bun not installed in this runner.

HAPI Bot

@tiann tiann merged commit 00ba610 into tiann:main Apr 3, 2026
2 checks passed
@junmo-kim junmo-kim deleted the feat/rate-limit-event-display branch April 3, 2026 00:33
Sign up for free to 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.

2 participants