Skip to content

fix(ai): raise task error/warning history cap from 50 to a named constant (#1414) #1568

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
meetp06 wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

fix(ai): raise task error/warning history cap from 50 to a named constant (#1414) #1568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d69e5a4
fix(ai): raise task error/warning history cap from 50 to a named cons…
meetp06 Jul 13, 2026
c50bdcf
test(ai): make status-history cap test track CONST_STATUS_HISTORY_LIMIT
meetp06 Jul 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/ai/src/ai/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
CONST_READY_POLL_INTERVAL = 0.250 # seconds between readiness checks
CONST_SUBPROCESS_BUFFER_LIMIT = 16 * 1024 * 1024 # bytes for subprocess stdin/stdout/stderr buffers (16MB)
CONST_STATUS_UPDATE_CANCEL_TIMEOUT = 2.0 # seconds to wait for status update task cancellation
CONST_STATUS_HISTORY_LIMIT = 1000 # max error/warning messages retained per task in memory (was 50; see #1414)
CONST_DEFAULT_TTL = 15 * 60 # default time-to-live for idle tasks in seconds (15 minutes)
CONST_TTL_CHECK = 60 # check for tasks to kill every 60 seconds

Expand Down
9 changes: 5 additions & 4 deletions packages/ai/src/ai/modules/task/task_engine.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
CONST_READY_POLL_INTERVAL,
CONST_SUBPROCESS_BUFFER_LIMIT,
CONST_STATUS_UPDATE_CANCEL_TIMEOUT,
CONST_STATUS_HISTORY_LIMIT,
)
from ai import CONST_AI_NODE_SCRIPT
from ai.common.dap import DAPBase, DAPClient, TransportWebSocket
Expand Down Expand Up @@ -940,16 +941,16 @@ def _update_status(self, message: Dict[str, Any]) -> None:
error_message = body.get('message', '')
self._status.errors.append(error_message)

if len(self._status.errors) > 50:
self._status.errors = self._status.errors[-50:]
if len(self._status.errors) > CONST_STATUS_HISTORY_LIMIT:
self._status.errors = self._status.errors[-CONST_STATUS_HISTORY_LIMIT:]

# Handle warning messages with buffer management
elif event_type == 'apaevt_status_warning':
warning_message = body.get('message', '')
self._status.warnings.append(warning_message)

if len(self._status.warnings) > 50:
self._status.warnings = self._status.warnings[-50:]
if len(self._status.warnings) > CONST_STATUS_HISTORY_LIMIT:
self._status.warnings = self._status.warnings[-CONST_STATUS_HISTORY_LIMIT:]
Comment on lines +944 to +953

@coderabbitai coderabbitai Bot Jul 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Update the status-history tests for the new limit.

The existing error-buffer test still initializes 50 entries and asserts a final length of 50. With this implementation, it will contain 51 entries, so the test suite will fail. Update the test to use CONST_STATUS_HISTORY_LIMIT and seed limit entries before appending the new 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 `@packages/ai/src/ai/modules/task/task_engine.py` around lines 944 - 953,
Update the status-history error-buffer test to import and use
CONST_STATUS_HISTORY_LIMIT, seed exactly limit existing entries, then append the
new error and assert the buffer is trimmed to the configured limit rather than a
hard-coded 50.


# Handle download progress
elif event_type == 'apaevt_status_download':
Expand Down
11 changes: 6 additions & 5 deletions packages/ai/tests/ai/modules/task/test_task_engine.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

import pytest

from ai.constants import CONST_STATUS_HISTORY_LIMIT
from ai.modules.task.task_engine import Task


Expand Down Expand Up @@ -470,24 +471,24 @@ def test_update_status_error_event_appends_to_errors():
assert t._status.errors == ['disk full']


def test_update_status_errors_buffer_trims_to_50():
"""Error buffer keeps only the most recent 50 entries."""
def test_update_status_errors_buffer_trims_to_limit():
"""Error buffer keeps only the most recent CONST_STATUS_HISTORY_LIMIT entries."""
t = _task(status=_make_status_for_update())
t._status.errors = [f'err-{i}' for i in range(50)]
t._status.errors = [f'err-{i}' for i in range(CONST_STATUS_HISTORY_LIMIT)]
Task._update_status(
t,
{
'event': 'apaevt_status_error',
'body': {'message': 'err-new'},
},
)
assert len(t._status.errors) == 50
assert len(t._status.errors) == CONST_STATUS_HISTORY_LIMIT
assert t._status.errors[-1] == 'err-new'
assert 'err-0' not in t._status.errors # oldest evicted


def test_update_status_warning_event_appends_and_trims():
"""An ``apaevt_status_warning`` event appends to warnings with the same 50-cap."""
"""An ``apaevt_status_warning`` event appends to warnings with the same history cap."""
Comment on lines 490 to +491

@coderabbitai coderabbitai Bot Jul 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Align the test name and docstring with its implementation, or add trimming assertions.

The test is named test_update_status_warning_event_appends_and_trims and its docstring mentions the history cap, but the implementation only asserts that a single warning is appended. It does not actually test the trimming behavior.

Consider renaming the test to match the error tests (e.g., test_update_status_warning_event_appends_to_warnings) and adding a dedicated trimming test for warnings to ensure complete coverage.

♻️ Proposed refactor 
-def test_update_status_warning_event_appends_and_trims():
-    """An ``apaevt_status_warning`` event appends to warnings with the same history cap."""
+def test_update_status_warning_event_appends_to_warnings():
+    """An ``apaevt_status_warning`` event appends to ``status.warnings``."""

To fully cover the trimming behavior for warnings, you can add this new test alongside it:

def test_update_status_warnings_buffer_trims_to_limit():
    """Warning buffer keeps only the most recent CONST_STATUS_HISTORY_LIMIT entries."""
    t = _task(status=_make_status_for_update())
    t._status.warnings = [f'warn-{i}' for i in range(CONST_STATUS_HISTORY_LIMIT)]
    Task._update_status(
        t,
        {
            'event': 'apaevt_status_warning',
            'body': {'message': 'warn-new'},
        },
    )
    assert len(t._status.warnings) == CONST_STATUS_HISTORY_LIMIT
    assert t._status.warnings[-1] == 'warn-new'
    assert 'warn-0' not in t._status.warnings  # oldest evicted
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def test_update_status_warning_event_appends_and_trims():
"""An ``apaevt_status_warning`` event appends to warnings with the same 50-cap."""
"""An ``apaevt_status_warning`` event appends to warnings with the same history cap."""
def test_update_status_warning_event_appends_to_warnings():
"""An ``apaevt_status_warning`` event appends to ``status.warnings``."""
🤖 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 `@packages/ai/tests/ai/modules/task/test_task_engine.py` around lines 490 -
491, Rename test_update_status_warning_event_appends_and_trims and its docstring
to describe only appending a warning, matching the existing implementation and
analogous error tests. Add a separate warning-buffer trimming test around
Task._update_status that fills warnings to CONST_STATUS_HISTORY_LIMIT, appends a
new warning, and verifies the limit is preserved and the oldest entry is
evicted.

t = _task(status=_make_status_for_update())
Task._update_status(
t,
Expand Down
6 changes: 4 additions & 2 deletions packages/client-python/src/rocketride/types/task.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,10 +360,12 @@ class TASK_STATUS(BaseModel):

# Error and Warning Management with History
warnings: List[str] = Field(
default_factory=list, description='Warning message history (limited to 50 recent entries)'
default_factory=list, description='Warning message history (limited to 1000 recent entries)'
)

errors: List[str] = Field(default_factory=list, description='Error message history (limited to 50 recent entries)')
errors: List[str] = Field(
default_factory=list, description='Error message history (limited to 1000 recent entries)'
)

# Current Processing Context
currentObject: str = Field(default='', description='Name/identifier of the item currently being processed')
Expand Down
Loading