Skip to content

Test parser activity status rows#67

Merged
TraderSamwise merged 1 commit into
masterfrom
test/chat-parser-activity-spinners
Jun 6, 2026
Merged

Test parser activity status rows#67
TraderSamwise merged 1 commit into
masterfrom
test/chat-parser-activity-spinners

Conversation

@TraderSamwise

@TraderSamwise TraderSamwise commented Jun 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • replace the parser's narrow activity verb list with a duration/ellipsis-shaped activity detector
  • add contract coverage that activity/progress rows remain status text with exact wording preserved
  • add Claude/Codex fixtures for multi-unit durations, unknown activity verbs, ellipsis rows, and dash status rows

Verification

  • /Users/sam/cs/aimux/node_modules/.bin/vitest run src/agent-output-parser-fixtures.test.ts src/agent-output-parser.test.ts src/context/context-bridge.test.ts
  • /Users/sam/cs/aimux/node_modules/.bin/tsc --noEmit
  • PATH=/Users/sam/cs/aimux/node_modules/.bin:/Users/sam/.local/volta-shims:/Users/sam/.local/bin:/Users/sam/.bun/bin:/Users/sam/.codex/tmp/arg0/codex-arg0MLlwy8:/Users/sam/.volta/tools/image/packages/@openai/codex/lib/node_modules/@openai/codex/node_modules/@openai/codex-darwin-arm64/vendor/aarch64-apple-darwin/codex-path:/opt/homebrew/opt/mysql-client/bin:/Users/sam/opt/anaconda3/envs/py39/bin:/Users/sam/.npm-global/bin:/Users/sam/pathother:/Users/sam/apache-ant/bin:/usr/local/mysql/bin:/Users/sam/.codex/tmp/arg0/codex-arg0FiIdYh:/Applications/cmux.app/Contents/Resources/bin:/Users/sam/.nvm/versions/node/v24.16.0/bin:/Users/sam/opt/anaconda3/condabin:/Users/Sam/.npm-global/bin:/Users/Sam/pathother:/Users/Sam/apache-ant/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/pkg/env/global/bin:/Library/Apple/usr/bin:/usr/local/share/dotnet:/Users/sam/.cargo/bin:/Users/sam/bin yarn lint
  • PATH=/Users/sam/cs/aimux/node_modules/.bin:/Users/sam/.local/volta-shims:/Users/sam/.local/bin:/Users/sam/.bun/bin:/Users/sam/.codex/tmp/arg0/codex-arg0MLlwy8:/Users/sam/.volta/tools/image/packages/@openai/codex/lib/node_modules/@openai/codex/node_modules/@openai/codex-darwin-arm64/vendor/aarch64-apple-darwin/codex-path:/opt/homebrew/opt/mysql-client/bin:/Users/sam/opt/anaconda3/envs/py39/bin:/Users/sam/.npm-global/bin:/Users/sam/pathother:/Users/sam/apache-ant/bin:/usr/local/mysql/bin:/Users/sam/.codex/tmp/arg0/codex-arg0FiIdYh:/Applications/cmux.app/Contents/Resources/bin:/Users/sam/.nvm/versions/node/v24.16.0/bin:/Users/sam/opt/anaconda3/condabin:/Users/Sam/.npm-global/bin:/Users/Sam/pathother:/Users/Sam/apache-ant/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/pkg/env/global/bin:/Library/Apple/usr/bin:/usr/local/share/dotnet:/Users/sam/.cargo/bin:/Users/sam/bin yarn build
  • PATH=/Users/sam/cs/aimux/node_modules/.bin:/Users/sam/.local/volta-shims:/Users/sam/.local/bin:/Users/sam/.bun/bin:/Users/sam/.codex/tmp/arg0/codex-arg0MLlwy8:/Users/sam/.volta/tools/image/packages/@openai/codex/lib/node_modules/@openai/codex/node_modules/@openai/codex-darwin-arm64/vendor/aarch64-apple-darwin/codex-path:/opt/homebrew/opt/mysql-client/bin:/Users/sam/opt/anaconda3/envs/py39/bin:/Users/sam/.npm-global/bin:/Users/sam/pathother:/Users/sam/apache-ant/bin:/usr/local/mysql/bin:/Users/sam/.codex/tmp/arg0/codex-arg0FiIdYh:/Applications/cmux.app/Contents/Resources/bin:/Users/sam/.nvm/versions/node/v24.16.0/bin:/Users/sam/opt/anaconda3/condabin:/Users/Sam/.npm-global/bin:/Users/Sam/pathother:/Users/Sam/apache-ant/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/pkg/env/global/bin:/Library/Apple/usr/bin:/usr/local/share/dotnet:/Users/sam/.cargo/bin:/Users/sam/bin yarn test

Summary by CodeRabbit

  • New Features

    • Enhanced parsing and detection of activity/progress status lines to better preserve exact activity wording across various formats, including duration patterns and alternative bullet styles.
  • Tests

    • Added comprehensive test coverage for activity/progress status patterns across multiple AI model outputs.

@vercel

vercel Bot commented Jun 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
app Ready Ready Preview, Comment Jun 6, 2026 8:10am

@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown

Linter diff in the way? Review this PR in Change Stack to focus on meaningful changes and expand context only when needed.

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds a new contract invariant requiring activity/progress rows to remain classified as status text while preserving their exact wording. The parser refactors activity/progress detection into shared regex helpers and applies them consistently across status-line classification and active-work detection. Test fixtures extend coverage with multiple Claude/Codex activity patterns.

Changes

Activity/Progress Status Parsing Invariant and Refactor

Layer / File(s) Summary
Contract invariant and test assertion
src/agent-output-parser-contract.ts, src/agent-output-parser-fixtures.test.ts
A new invariant is added to AGENT_OUTPUT_PARSER_CONTRACT.invariants requiring activity/progress rows to remain status text with exact activity wording preserved, and the test suite asserts this requirement.
Activity progress detection helpers
src/agent-output-parser.ts
New regex constants and a looksLikeActivityProgressText(text) helper function are introduced to centralize detection of activity/progress phrasing including "for " forms, parenthetical durations, and ellipsis variants.
Parser status line detection refactor
src/agent-output-parser.ts
isStatusLine() is rewritten to extract bullet text and use looksLikeActivityProgressText() across multiple prefix styles; stripStatusMarker regex is broadened to include - bullets; and looksLikeActiveWorkStatus() is updated to use the shared helper.
Test fixtures for activity and status patterns
src/agent-output-parser-fixtures.ts
Existing fixture reformatted and four new test cases added (claude-multi-unit-activity-status, codex-unknown-activity-verb-status, codex-ellipsis-activity-status, codex-dash-activity-status) covering Claude/Codex activity and status patterns with varying duration and ellipsis markers.

Possibly Related PRs

  • TraderSamwise/aimux#57: Modifies isStatusLine and status marker-stripping heuristics in the parser's line-identification layer.
  • TraderSamwise/aimux#54: Changes how status/progress markers are detected and classified in src/agent-output-parser.ts, directly related to this PR's status parsing refactor.
  • TraderSamwise/aimux#66: Introduces the contract/fixture framework that this PR extends with the new invariant and activity/status pattern fixtures.

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Activity flows in many forms—dash, ellipsis, "for five seconds"—
But status text keeps its true wording, and the parser now heeds it.
One helper function, shared across the realm, catches them all,
Making progress classification steady, fair, and faithful. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Test parser activity status rows' directly reflects the main changes: adding tests and contract coverage for activity/progress status row detection in the agent output parser.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 test/chat-parser-activity-spinners

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/agent-output-parser.ts (1)

245-253: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Normalize bullet-prefixed status lines before active-work checks.

At Line 253, looksLikeActiveWorkStatus evaluates raw trimmed lines. For -prefixed status rows, looksLikeActivityProgressText() won’t match (it expects the activity verb at the start), and ^Starting MCP servers also misses due the leading bullet. That can incorrectly set hasActiveWorkBeforeNextTurn and skew prompt/status normalization.

Suggested fix
   const looksLikeActiveWorkStatus = (text: string) =>
     String(text || "")
       .split("\n")
       .some((line) => {
-        const trimmed = line.trim();
+        const trimmed = line.trim();
+        const normalized = trimmed.replace(/^(?:[•*-]|[✻✽✶])\s+/, "");
         return (
-          /\bWorking \(\d+s\b.*\besc to interrupt\b/i.test(trimmed) ||
-          /^Starting MCP servers\b/i.test(trimmed) ||
-          looksLikeActivityProgressText(trimmed)
+          /\bWorking \(\d+s\b.*\besc to interrupt\b/i.test(normalized) ||
+          /^Starting MCP servers\b/i.test(normalized) ||
+          looksLikeActivityProgressText(normalized)
         );
       });
🤖 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 `@src/agent-output-parser.ts` around lines 245 - 253, The active-work detection
in looksLikeActiveWorkStatus tests the raw trimmed line, so lines that start
with a bullet (e.g., "•", "-", "*", U+2022) won't match starts-with patterns or
looksLikeActivityProgressText; normalize each trimmed line by stripping any
common bullet characters plus following whitespace (e.g., use a regex to remove
leading [•\-\*\u2022]\s*), assign that to a variable like normalizedLine, and
run the existing checks (/\\bWorking \\(\\d+s\\b.*\\besc to interrupt\\b/i.test,
/^Starting MCP servers\\b/i.test, and looksLikeActivityProgressText) against
normalizedLine instead of trimmed so bullet-prefixed status rows are correctly
detected by looksLikeActiveWorkStatus.
🤖 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 `@src/agent-output-parser.ts`:
- Around line 245-253: The active-work detection in looksLikeActiveWorkStatus
tests the raw trimmed line, so lines that start with a bullet (e.g., "•", "-",
"*", U+2022) won't match starts-with patterns or looksLikeActivityProgressText;
normalize each trimmed line by stripping any common bullet characters plus
following whitespace (e.g., use a regex to remove leading [•\-\*\u2022]\s*),
assign that to a variable like normalizedLine, and run the existing checks
(/\\bWorking \\(\\d+s\\b.*\\besc to interrupt\\b/i.test, /^Starting MCP
servers\\b/i.test, and looksLikeActivityProgressText) against normalizedLine
instead of trimmed so bullet-prefixed status rows are correctly detected by
looksLikeActiveWorkStatus.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2ab4b861-fab6-4953-91aa-74816c19c18f

📥 Commits

Reviewing files that changed from the base of the PR and between 8866824 and e12a713.

📒 Files selected for processing (4)
  • src/agent-output-parser-contract.ts
  • src/agent-output-parser-fixtures.test.ts
  • src/agent-output-parser-fixtures.ts
  • src/agent-output-parser.ts

@TraderSamwise
TraderSamwise merged commit 2021112 into master Jun 6, 2026
3 checks passed
@TraderSamwise
TraderSamwise deleted the test/chat-parser-activity-spinners branch June 6, 2026 08:20
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.

1 participant