Skip to content

fix: React agent regex false positives, dead code, and test gaps #463

Description

@avoidwork

Summary

The React agent in src/agent/react.js has a medium-severity regex false positive in isContextLengthError that can trigger unnecessary compaction on rate limit errors, several low-severity issues including a dead stepTimeout property and redundant recursionLimit parameter, and significant test coverage gaps across cache hits, version parameters, and timeout assignments.

Environment

  • OS: Linux 7.0.2-7-pve
  • Node.js: v25.8.1
  • madz version: 1.21.0
  • LLM provider: Unknown — user to confirm

Reproduction

  1. Audit src/agent/react.js against LangChain/LangGraph v2 APIs and project conventions
  2. Observe that isContextLengthError in src/tools/compact_context.js:10 uses regex /limit[:\s]*(\d+)/i
  3. Trigger a rate limit error (e.g., "rate limit: 429") — the regex falsely matches as a context length error
  4. Observe that agent.stepTimeout is set but never consumed by LangGraph's streamEvents
  5. Observe that recursionLimit is passed at both graph construction and invocation time
  6. Note that turnHashWindow and turnBufferMax options lack JSDoc documentation
  7. Note that turnHashes sliding window eviction uses O(n) Set iteration

Expected Behavior

  • isContextLengthError should only match context length errors, not rate limit or other "limit" errors
  • agent.stepTimeout should either be consumed by LangGraph or removed
  • recursionLimit should be passed in only one place
  • All options should be documented in JSDoc
  • Test suite should cover cache hits, version parameters, timeout assignments, and recursion limits

Actual Behavior

  • The regex /limit[:\s]*(\d+)/i matches any error containing "limit: 1234", causing false positives on rate limit errors and triggering unnecessary compaction attempts
  • agent.stepTimeout = timeout is set on the compiled graph but LangGraph's streamEvents does not natively read this property — it's effectively a no-op
  • recursionLimit is passed to both createReactAgentGraph (line 100) and streamEvents (line 212), with LangGraph using the invocation-time value, making the construction-time value redundant
  • turnHashWindow and turnBufferMax are read from options but not documented in the JSDoc of callReactAgent
  • turnHashes.delete(turnHashes.keys().next().value) iterates the Set to find the first key — O(n) eviction
  • No tests exist for cache hit path, streamEvents version parameter, agent.stepTimeout assignment, recursionLimit in streamEvents options, or false positive detection in isContextLengthError

Additional Context

Audit Strengths

  • Clean separation between callReactAgent (wrapper) and callReactAgentStreaming (core logic)
  • Compaction retry loop with MAX_COMPACTION_ITERATIONS is well-implemented
  • Turn-level hashing for loop detection is a clever approach
  • Callback event model is well-designed for both TUI and non-TUI consumers

Suggested Labels

bug, testing, documentation, langgraph


Audit Findings

Confirmed code paths:

  • src/tools/compact_context.js:10CONTEXT_LENGTH_PATTERN_2 = /limit[:\s]*(\d+)/i is confirmed. This pattern matches any error containing "limit: 1234", including rate limit errors like "rate limit: 429". The fix should require "context" or "tokens" in the match, e.g., /context.*limit[:\s]*(\d+)/i or /limit[:\s]*(\d+).*tokens/i.
  • src/agent/react.js:103agent.stepTimeout = timeout is set but grep confirms it is never read anywhere else in the codebase. It is effectively dead code unless LangGraph reads it internally (unconfirmed).
  • src/agent/react.js:100 and src/agent/react.js:212recursionLimit is passed to both createReactAgentGraph at construction and to streamEvents at invocation. LangGraph uses the invocation-time value, making the construction-time value redundant.
  • src/agent/react.js:267turnHashes.delete(turnHashes.keys().next().value) confirms O(n) Set iteration for sliding window eviction. At window=20 this is negligible, but a Set does not guarantee insertion order iteration in all JS engines.
  • src/agent/react.js:244-245turnHashWindow and turnBufferMax are read from options but not documented in the JSDoc of callReactAgent.

Test coverage gaps confirmed:

  • No test for cache hit path (lines 222-231)
  • No test for streamEvents version parameter (line 279 passes version: "v2")
  • No test for agent.stepTimeout assignment (line 103)
  • No test for recursionLimit in streamEvents options
  • No test for isContextLengthError false positive detection

Suggested fix priority:

  1. Medium: Narrow CONTEXT_LENGTH_PATTERN_2 regex to avoid false positives
  2. Low: Remove or document agent.stepTimeout
  3. Low: Remove redundant recursionLimit at construction time
  4. Low: Add JSDoc for turnHashWindow and turnBufferMax
  5. Low: Consider using a queue-based approach for turnHashes eviction if window size grows

Metadata

Metadata

Assignees

No one assigned

    Labels

    approvedAn identifier for Madz to take action.bugSomething isn't workingin progress

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions