feat: add manual retry state apis #405
Conversation
- Introduced a new API endpoint for manual state retries, allowing users to trigger retries for specific states. - Implemented request and response models for manual retry operations. - Added error handling for duplicate retry states and invalid API keys. - Enhanced logging for better traceability of retry operations. This feature improves the system's ability to manage state retries effectively.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds manual retry capability: new Pydantic request/response models, an async controller to create retry states, and a POST route at /v0/namespace/{namespace_name}/state/{state_id}/manual-retry. Includes unit tests for models, route behavior, and controller scenarios (not found, duplicate key, DB errors). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as Routes (FastAPI)
participant C as Controller: manual_retry_state
participant DB as Database (Beanie/Mongo)
Client->>API: POST /v0/namespace/{ns}/state/{state_id}/manual-retry { fanout_id }
API->>API: Validate API key
API->>API: Get or gen x_exosphere_request_id
API->>C: manual_retry_state(ns, PydanticObjectId(state_id), body, req_id)
C->>DB: Find original State by id
alt State not found
C-->>API: HTTP 404
API-->>Client: 404 Not Found
else Found
C->>DB: Insert retry State (status=CREATED, fanout_id from body)
alt Duplicate key
C-->>API: HTTP 409 Duplicate retry
API-->>Client: 409 Conflict
else Inserted
C->>DB: Update original State status=RETRY_CREATED
C-->>API: ManualRetryResponseModel { id, status }
API-->>Client: 200 OK with { id, status }
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @NiveditJain, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a new feature that enables manual retries for specific states within the system. It provides a dedicated API endpoint, along with the necessary backend logic, data models, and robust error handling to facilitate effective management and traceability of these retry operations.
Highlights
- New API Endpoint: Introduced a new API endpoint
/state/{state_id}/manual-retryto allow users to trigger manual retries for specific states. - Manual Retry Logic: Implemented core logic for manual state retries, including creating a new state with
CREATEDstatus, updating the original state, and handling duplicate retry attempts. - Data Models: Defined
ManualRetryRequestModelandManualRetryResponseModelfor structured input and output of the manual retry API. - Error Handling & Logging: Added error handling for cases like state not found, duplicate retry states, and invalid API keys, along with enhanced logging for traceability.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Code Review
This pull request introduces a new API endpoint for manually retrying a state. The implementation is well-structured, with a new controller, models, and route. My review includes several suggestions for improvement. I've pointed out a critical correctness issue where states in any status can be retried, which could lead to data inconsistencies; I recommend restricting retries to error states. Other feedback includes improving exception handling to preserve stack traces, fixing a typo, ensuring URL path consistency, and suggesting a refactor to reduce duplicated code for API key validation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
state-manager/app/controller/manul_retry_state.py(1 hunks)state-manager/app/models/manual_retry.py(1 hunks)state-manager/app/routes.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
state-manager/app/controller/manul_retry_state.py (4)
state-manager/app/models/manual_retry.py (2)
ManualRetryRequestModel(5-6)ManualRetryResponseModel(9-11)state-manager/app/singletons/logs_manager.py (2)
LogsManager(9-66)get_logger(65-66)state-manager/app/models/state_status_enum.py (1)
StateStatusEnum(4-20)state-manager/app/models/db/state.py (1)
State(13-104)
state-manager/app/models/manual_retry.py (1)
state-manager/app/models/state_status_enum.py (1)
StateStatusEnum(4-20)
state-manager/app/routes.py (3)
state-manager/app/models/manual_retry.py (2)
ManualRetryRequestModel(5-6)ManualRetryResponseModel(9-11)state-manager/app/controller/manul_retry_state.py (1)
manual_retry_state(12-49)state-manager/app/utils/check_secret.py (1)
check_api_key(11-16)
🔇 Additional comments (1)
state-manager/app/models/manual_retry.py (1)
9-11: Response model looks good.Fields align with controller return; no issues.
…nition - Fixed the import path for the manual retry state controller in routes.py. - Updated the route definition for manual retry to include a leading slash for consistency. - Added a new controller file for manual retry state functionality, implementing the logic for handling manual retries. - Introduced unit tests for the manual retry request and response models, ensuring validation and functionality. - Enhanced unit tests for the manual retry state route, covering various scenarios including valid and invalid API keys. These changes improve the structure and reliability of the manual retry feature.
- Updated the query for fetching the state to include the namespace name, ensuring accurate state retrieval. - Changed the HTTP status code for duplicate retry state errors from 400 to 409 to better reflect the conflict nature of the error. - Simplified exception handling by removing the unnecessary re-raise of the caught exception. These changes enhance the reliability and clarity of the manual retry state functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
state-manager/app/controller/manual_retry_state.py (1)
12-50: Add input validation for state_id and consider retry count tracking.The implementation looks good overall, but consider these improvements:
- The function should validate that
state_idis a valid ObjectId before querying- Consider incrementing
retry_counton the new retry state to track retry attempts- The error message in line 48 is missing the exception details
Apply this diff to address the issues:
async def manual_retry_state(namespace_name: str, state_id: PydanticObjectId, body: ManualRetryRequestModel, x_exosphere_request_id: str): try: + # Validate state_id format + if not state_id: + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid state ID") + logger.info(f"Manual retry state {state_id} for namespace {namespace_name}", x_exosphere_request_id=x_exosphere_request_id) state = await State.find_one(State.id == state_id) if not state: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="State not found") try: retry_state = State( node_name=state.node_name, namespace_name=state.namespace_name, identifier=state.identifier, graph_name=state.graph_name, run_id=state.run_id, status=StateStatusEnum.CREATED, inputs=state.inputs, outputs={}, error=None, parents=state.parents, does_unites=state.does_unites, + retry_count=state.retry_count + 1, # Track retry attempts fanout_id=body.fanout_id # this will ensure that multiple unwanted retries are not formed because of index in database ) retry_state = await retry_state.insert() logger.info(f"Retry state {retry_state.id} created for state {state_id}", x_exosphere_request_id=x_exosphere_request_id) state.status = StateStatusEnum.RETRY_CREATED await state.save() return ManualRetryResponseModel(id=str(retry_state.id), status=retry_state.status) except DuplicateKeyError: logger.info(f"Duplicate retry state detected for state {state_id}. A retry state with the same unique key already exists.", x_exosphere_request_id=x_exosphere_request_id) raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Duplicate retry state detected") except Exception as e: - logger.error(f"Error manual retry state {state_id} for namespace {namespace_name}", x_exosphere_request_id=x_exosphere_request_id) + logger.error(f"Error manual retry state {state_id} for namespace {namespace_name}: {str(e)}", x_exosphere_request_id=x_exosphere_request_id, exc_info=True) raise e
♻️ Duplicate comments (3)
state-manager/app/routes.py (3)
183-189: Route decorator path is correct.The route path
/state/{state_id}/manual-retryis properly formatted with the leading slash. The previous review comments about the missing leading slash appear to be outdated as the current implementation is correct.
186-186: Consider using HTTP 201 CREATED for new retry state creation.Since this endpoint creates a new retry state resource, HTTP 201 CREATED would be more semantically appropriate than HTTP 200 OK.
- status_code=status.HTTP_200_OK, + status_code=status.HTTP_201_CREATED,
187-187: Fix grammatical error in response description.The response description has a grammatical error.
- response_description="State manual retry successfully", + response_description="State manual retry triggered successfully",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
state-manager/app/controller/manual_retry_state.py(1 hunks)state-manager/app/routes.py(2 hunks)state-manager/tests/unit/models/test_manual_retry.py(1 hunks)state-manager/tests/unit/test_routes.py(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
state-manager/tests/unit/models/test_manual_retry.py (2)
state-manager/app/models/manual_retry.py (2)
ManualRetryRequestModel(5-6)ManualRetryResponseModel(9-11)state-manager/app/models/state_status_enum.py (1)
StateStatusEnum(4-20)
state-manager/app/controller/manual_retry_state.py (4)
state-manager/app/models/manual_retry.py (2)
ManualRetryRequestModel(5-6)ManualRetryResponseModel(9-11)state-manager/app/singletons/logs_manager.py (2)
LogsManager(9-66)get_logger(65-66)state-manager/app/models/state_status_enum.py (1)
StateStatusEnum(4-20)state-manager/app/models/db/state.py (1)
State(13-104)
state-manager/tests/unit/test_routes.py (3)
state-manager/app/models/manual_retry.py (2)
ManualRetryRequestModel(5-6)ManualRetryResponseModel(9-11)state-manager/app/models/state_status_enum.py (1)
StateStatusEnum(4-20)state-manager/app/routes.py (2)
get_node_run_details_route(372-381)manual_retry_state_route(190-199)
state-manager/app/routes.py (3)
state-manager/app/models/manual_retry.py (2)
ManualRetryRequestModel(5-6)ManualRetryResponseModel(9-11)state-manager/app/controller/manual_retry_state.py (1)
manual_retry_state(12-49)state-manager/app/utils/check_secret.py (1)
check_api_key(11-16)
🔇 Additional comments (9)
state-manager/tests/unit/models/test_manual_retry.py (1)
1-241: Comprehensive test coverage!The test suite thoroughly validates both
ManualRetryRequestModelandManualRetryResponseModelwith excellent edge case coverage including empty strings, very long strings, special characters, and validation errors. The tests properly verify Pydantic's validation behavior and serialization.state-manager/app/routes.py (2)
53-56: LGTM!The imports for manual retry models and controller are correctly added.
190-199: Route implementation follows established patterns.The route handler correctly:
- Validates the API key and logs appropriately
- Extracts or generates the request ID
- Converts the state_id string to PydanticObjectId
- Delegates to the controller function
The implementation is consistent with other route handlers in the file.
state-manager/tests/unit/test_routes.py (6)
11-11: LGTM!The import for manual retry models is correctly added to support the new test cases.
36-36: Route path assertion correctly validates the manual retry endpoint.The test properly verifies that the manual retry route is registered with the expected path.
278-296: Well-structured model validation tests.The tests properly validate both request and response models with appropriate test data and assertions.
320-321: Route handler imports correctly updated.The manual_retry_state_route is properly added to the imports verification.
339-339: Route handler callable check correctly added.The test properly verifies that manual_retry_state_route is callable.
1062-1120: Comprehensive route handler tests!The test suite thoroughly covers the manual retry route with:
- Valid API key scenario with proper argument forwarding
- Invalid API key handling with 401 response
- Request ID generation when not present
- Proper PydanticObjectId conversion verification
The tests follow the established patterns and provide excellent coverage.
- Introduced a new test file for the manual retry state, covering various scenarios including successful state creation, error handling for not found states, and duplicate key errors. - Enhanced tests to verify logging, database error handling, and preservation of original state fields during retries. - Updated the README to include the new test file and detailed coverage of the manual retry state functionality. These changes improve the test coverage and reliability of the manual retry state feature.
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
state-manager/tests/unit/test_routes.py (1)
354-358: Fixture may raise AttributeError; ensure request.state exists instead of delattr.
Creating .state explicitly avoids deleting a non-existent attribute on a MagicMock.Apply:
- def mock_request_no_id(self): - """Mock request object without request_id""" - request = MagicMock() - delattr(request.state, 'x_exosphere_request_id') - return request + def mock_request_no_id(self): + """Mock request object without request_id""" + request = MagicMock() + request.state = MagicMock() # state exists but has no x_exosphere_request_id + return request
♻️ Duplicate comments (1)
state-manager/app/controller/manual_retry_state.py (1)
21-34: Enqueue timing: preserve or make configurable.
Consider preserving original enqueue_after or allowing a delay in the request model. Otherwise all manual retries enqueue “now” by default.Example:
- retry_state = State( + retry_state = State( ... - does_unites=state.does_unites, + does_unites=state.does_unites, + enqueue_after=state.enqueue_after, # or compute based on optional body.retry_delay_ms fanout_id=body.fanout_id )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
state-manager/app/controller/manual_retry_state.py(1 hunks)state-manager/tests/README.md(2 hunks)state-manager/tests/unit/controller/test_manual_retry_state.py(1 hunks)state-manager/tests/unit/test_routes.py(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
state-manager/app/controller/manual_retry_state.py (4)
state-manager/app/models/manual_retry.py (2)
ManualRetryRequestModel(5-6)ManualRetryResponseModel(9-11)state-manager/app/singletons/logs_manager.py (2)
LogsManager(9-66)get_logger(65-66)state-manager/app/models/state_status_enum.py (1)
StateStatusEnum(4-20)state-manager/app/models/db/state.py (1)
State(13-104)
state-manager/tests/unit/controller/test_manual_retry_state.py (3)
state-manager/app/controller/manual_retry_state.py (1)
manual_retry_state(12-49)state-manager/app/models/manual_retry.py (2)
ManualRetryRequestModel(5-6)ManualRetryResponseModel(9-11)state-manager/app/models/state_status_enum.py (1)
StateStatusEnum(4-20)
state-manager/tests/unit/test_routes.py (3)
state-manager/app/models/manual_retry.py (2)
ManualRetryRequestModel(5-6)ManualRetryResponseModel(9-11)state-manager/app/models/state_status_enum.py (1)
StateStatusEnum(4-20)state-manager/app/routes.py (2)
get_node_run_details_route(372-381)manual_retry_state_route(190-199)
🪛 markdownlint-cli2 (0.17.2)
state-manager/tests/README.md
84-84: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
85-85: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
98-98: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (4)
state-manager/tests/README.md (1)
17-17: Docs entry for new tests: LGTM.
Accurately lists the new controller test file.state-manager/tests/unit/test_routes.py (3)
11-11: Imports for manual retry models: LGTM.
Models are referenced in new tests below.
36-36: Route path assertion: LGTM.
Covers the new manual retry endpoint path.
1062-1120: Manual retry route tests: LGTM.
Validate API key gating, request-id propagation, and arg forwarding including state_id casting.
| retry_state = State( | ||
| node_name=state.node_name, | ||
| namespace_name=state.namespace_name, | ||
| identifier=state.identifier, | ||
| graph_name=state.graph_name, | ||
| run_id=state.run_id, | ||
| status=StateStatusEnum.CREATED, | ||
| inputs=state.inputs, | ||
| outputs={}, | ||
| error=None, | ||
| parents=state.parents, | ||
| does_unites=state.does_unites, | ||
| fanout_id=body.fanout_id # this will ensure that multiple unwanted retries are not formed because of index in database | ||
| ) |
There was a problem hiding this comment.
Blocker: fingerprint collision for does_unites states; set retry_count on the new retry state.
Without bumping retry_count, the retry state’s fingerprint can equal the original (unique index uniq_state_fingerprint_unites), causing DuplicateKeyError even on first retry. Also aligns with the unique (node, ns, graph, identifier, run_id, retry_count, fanout_id) index.
Apply:
retry_state = State(
node_name=state.node_name,
namespace_name=state.namespace_name,
identifier=state.identifier,
graph_name=state.graph_name,
run_id=state.run_id,
status=StateStatusEnum.CREATED,
inputs=state.inputs,
outputs={},
error=None,
parents=state.parents,
does_unites=state.does_unites,
+ retry_count=(getattr(state, "retry_count", 0) + 1),
fanout_id=body.fanout_id # this will ensure that multiple unwanted retries are not formed because of index in database
)📝 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.
| retry_state = State( | |
| node_name=state.node_name, | |
| namespace_name=state.namespace_name, | |
| identifier=state.identifier, | |
| graph_name=state.graph_name, | |
| run_id=state.run_id, | |
| status=StateStatusEnum.CREATED, | |
| inputs=state.inputs, | |
| outputs={}, | |
| error=None, | |
| parents=state.parents, | |
| does_unites=state.does_unites, | |
| fanout_id=body.fanout_id # this will ensure that multiple unwanted retries are not formed because of index in database | |
| ) | |
| retry_state = State( | |
| node_name=state.node_name, | |
| namespace_name=state.namespace_name, | |
| identifier=state.identifier, | |
| graph_name=state.graph_name, | |
| run_id=state.run_id, | |
| status=StateStatusEnum.CREATED, | |
| inputs=state.inputs, | |
| outputs={}, | |
| error=None, | |
| parents=state.parents, | |
| does_unites=state.does_unites, | |
| retry_count=(getattr(state, "retry_count", 0) + 1), | |
| fanout_id=body.fanout_id # this will ensure that multiple unwanted retries are not formed because of index in database | |
| ) |
🤖 Prompt for AI Agents
In state-manager/app/controller/manual_retry_state.py around lines 21 to 34, the
newly created retry State does not set retry_count which can produce a
fingerprint collision against the uniq_state_fingerprint_unites index; set
retry_count on the new State to (state.retry_count or 0) + 1 so the retry
fingerprint differs from the original (handle None by treating as 0), e.g.
assign retry_count before persisting the new State.
| logger.info(f"Retry state {retry_state.id} created for state {state_id}", x_exosphere_request_id=x_exosphere_request_id) | ||
|
|
||
| state.status = StateStatusEnum.RETRY_CREATED | ||
| await state.save() | ||
|
|
||
| return ManualRetryResponseModel(id=str(retry_state.id), status=retry_state.status) |
There was a problem hiding this comment.
🧹 Nitpick
Consider atomicity between insert and original state update.
If save() fails after insert(), you end up with a retry state but original not marked RETRY_CREATED. Prefer a Mongo transaction or a compensating update with retry/backoff.
🤖 Prompt for AI Agents
In state-manager/app/controller/manual_retry_state.py around lines 36 to 41, the
code inserts a retry_state then sets state.status and calls state.save()
separately, which can leave data inconsistent if save() fails; wrap both the
insert and the original state update in a single MongoDB transaction (start a
client session and use with_transaction to insert the retry document and update
the original state status to RETRY_CREATED within the same transaction) so both
succeed or fail together; if transactions are not available in the deployment,
implement a compensating flow: after insert() attempt the state.save() with an
exponential backoff retry loop, and if all retries fail, delete the created
retry_state (or mark it as failed) to restore consistency, and ensure all
operations log errors with context and surface failures to the caller.
| except Exception as _: | ||
| logger.error(f"Error manual retry state {state_id} for namespace {namespace_name}", x_exosphere_request_id=x_exosphere_request_id) | ||
| raise |
There was a problem hiding this comment.
Don’t log HTTPException as server errors; use logger.exception for unexpected failures.
Current blanket except re-logs 404/409 as errors. Keep HTTPException untouched and add stacktrace for real failures.
Apply:
- except Exception as _:
- logger.error(f"Error manual retry state {state_id} for namespace {namespace_name}", x_exosphere_request_id=x_exosphere_request_id)
- raise
+ except HTTPException:
+ # propagate expected HTTP errors without error severity logging
+ raise
+ except Exception:
+ logger.exception(
+ f"Error creating manual retry state {state_id} for namespace {namespace_name}",
+ x_exosphere_request_id=x_exosphere_request_id,
+ )
+ raise📝 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.
| except Exception as _: | |
| logger.error(f"Error manual retry state {state_id} for namespace {namespace_name}", x_exosphere_request_id=x_exosphere_request_id) | |
| raise | |
| except HTTPException: | |
| # propagate expected HTTP errors without error severity logging | |
| raise | |
| except Exception: | |
| logger.exception( | |
| f"Error creating manual retry state {state_id} for namespace {namespace_name}", | |
| x_exosphere_request_id=x_exosphere_request_id, | |
| ) | |
| raise |
🤖 Prompt for AI Agents
In state-manager/app/controller/manual_retry_state.py around lines 47 to 49, the
current blanket except catches HTTPException and logs 404/409 as server errors;
change the handler to let HTTPException pass through unmodified (re-raise
immediately) and for all other exceptions use logger.exception to log the error
with stacktrace and context (include state_id, namespace_name,
x_exosphere_request_id), then re-raise the exception.
| ### 8. `manual_retry_state.py` | ||
| - ✅ Successful manual retry state creation | ||
| - ✅ State not found scenarios | ||
| - ✅ Duplicate retry state detection (DuplicateKeyError) | ||
| - ✅ Different fanout_id handling | ||
| - ✅ Complex inputs and multiple parents preservation | ||
| - ✅ Database errors during state lookup | ||
| - ✅ Database errors during state save | ||
| - ✅ Database errors during retry state insert | ||
| - ✅ Empty inputs and parents handling | ||
| - ✅ Namespace mismatch scenarios | ||
| - ✅ Field preservation and reset logic | ||
| - ✅ Logging verification | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick
Fix markdownlint errors around the new section.
Add blank lines around the heading and the list to satisfy MD022/MD032.
Apply:
-### 8. `manual_retry_state.py`
+
+### 8. `manual_retry_state.py`
+
- ✅ Successful manual retry state creation
- ✅ State not found scenarios
- ✅ Duplicate retry state detection (DuplicateKeyError)
- ✅ Different fanout_id handling
- ✅ Complex inputs and multiple parents preservation
- ✅ Database errors during state lookup
- ✅ Database errors during state save
- ✅ Database errors during retry state insert
- ✅ Empty inputs and parents handling
- ✅ Namespace mismatch scenarios
- ✅ Field preservation and reset logic
- ✅ Logging verification
+📝 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.
| ### 8. `manual_retry_state.py` | |
| - ✅ Successful manual retry state creation | |
| - ✅ State not found scenarios | |
| - ✅ Duplicate retry state detection (DuplicateKeyError) | |
| - ✅ Different fanout_id handling | |
| - ✅ Complex inputs and multiple parents preservation | |
| - ✅ Database errors during state lookup | |
| - ✅ Database errors during state save | |
| - ✅ Database errors during retry state insert | |
| - ✅ Empty inputs and parents handling | |
| - ✅ Namespace mismatch scenarios | |
| - ✅ Field preservation and reset logic | |
| - ✅ Logging verification | |
| ### 8. `manual_retry_state.py` | |
| - ✅ Successful manual retry state creation | |
| - ✅ State not found scenarios | |
| - ✅ Duplicate retry state detection (DuplicateKeyError) | |
| - ✅ Different fanout_id handling | |
| - ✅ Complex inputs and multiple parents preservation | |
| - ✅ Database errors during state lookup | |
| - ✅ Database errors during state save | |
| - ✅ Database errors during retry state insert | |
| - ✅ Empty inputs and parents handling | |
| - ✅ Namespace mismatch scenarios | |
| - ✅ Field preservation and reset logic | |
| - ✅ Logging verification | |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
84-84: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
85-85: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
🤖 Prompt for AI Agents
In state-manager/tests/README.md around lines 84 to 97, the new "### 8.
`manual_retry_state.py`" section is triggering markdownlint errors MD022/MD032
because there are no blank lines around the heading and the following list; add
a single blank line immediately before the "### 8. `manual_retry_state.py`"
heading, add one blank line between the heading and the start of the list, and
ensure there is a blank line after the end of the list so the heading and list
are each separated by one empty line from surrounding content.
| - ✅ Field preservation and reset logic | ||
| - ✅ Logging verification | ||
|
|
||
| ### 9. `upsert_graph_template.py` |
There was a problem hiding this comment.
🧹 Nitpick
Add a blank line before this heading.
Prevents MD022 violation.
Apply:
-### 9. `upsert_graph_template.py`
+
+### 9. `upsert_graph_template.py`📝 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.
| ### 9. `upsert_graph_template.py` | |
| ### 9. `upsert_graph_template.py` |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
98-98: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
In state-manager/tests/README.md around line 98, there is a missing blank line
immediately before the "### 9. `upsert_graph_template.py`" heading which
triggers an MD022 lint violation; insert a single empty line above that heading
so it is separated from the previous content.
| @pytest.fixture | ||
| def mock_original_state(self): | ||
| state = MagicMock() | ||
| state.id = PydanticObjectId() | ||
| state.node_name = "test_node" | ||
| state.namespace_name = "test_namespace" | ||
| state.identifier = "test_identifier" | ||
| state.graph_name = "test_graph" | ||
| state.run_id = "test_run_id" | ||
| state.status = StateStatusEnum.EXECUTED | ||
| state.inputs = {"key": "value"} | ||
| state.outputs = {"result": "success"} | ||
| state.error = "Original error" | ||
| state.parents = {"parent1": PydanticObjectId()} | ||
| state.does_unites = False | ||
| state.save = AsyncMock() | ||
| return state |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Set retry_count on the mocked original state.
Needed if controller increments retry_count; prevents TypeError when adding to MagicMock.
Apply:
state.outputs = {"result": "success"}
state.error = "Original error"
state.parents = {"parent1": PydanticObjectId()}
state.does_unites = False
+ state.retry_count = 0
state.save = AsyncMock()
return state🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "---- Searching for test file and retry_count usages ----"
rg -n --hidden --no-ignore "test_manual_retry_state.py" -S || true
rg -n --hidden --no-ignore "def mock_original_state" -S || true
rg -n --hidden --no-ignore "retry_count" -S || true
rg -n --hidden --no-ignore "\.retry_count" -S || true
TF="state-manager/tests/unit/controller/test_manual_retry_state.py"
if [ -f "$TF" ]; then
echo "---- Showing $TF (first 300 lines) ----"
sed -n '1,300p' "$TF"
else
echo "$TF not found at that exact path; listing matching files..."
rg -n --hidden --no-ignore "test_manual_retry_state.py" -S || true
fiLength of output: 15886
Set retry_count on the mocked original state.
Controller increments retry_count; set it on the fixture to avoid TypeError when adding to a MagicMock.
File: state-manager/tests/unit/controller/test_manual_retry_state.py
Lines: 33-49
state.outputs = {"result": "success"}
state.error = "Original error"
state.parents = {"parent1": PydanticObjectId()}
state.does_unites = False
+ state.retry_count = 0
state.save = AsyncMock()
return state📝 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.
| @pytest.fixture | |
| def mock_original_state(self): | |
| state = MagicMock() | |
| state.id = PydanticObjectId() | |
| state.node_name = "test_node" | |
| state.namespace_name = "test_namespace" | |
| state.identifier = "test_identifier" | |
| state.graph_name = "test_graph" | |
| state.run_id = "test_run_id" | |
| state.status = StateStatusEnum.EXECUTED | |
| state.inputs = {"key": "value"} | |
| state.outputs = {"result": "success"} | |
| state.error = "Original error" | |
| state.parents = {"parent1": PydanticObjectId()} | |
| state.does_unites = False | |
| state.save = AsyncMock() | |
| return state | |
| @pytest.fixture | |
| def mock_original_state(self): | |
| state = MagicMock() | |
| state.id = PydanticObjectId() | |
| state.node_name = "test_node" | |
| state.namespace_name = "test_namespace" | |
| state.identifier = "test_identifier" | |
| state.graph_name = "test_graph" | |
| state.run_id = "test_run_id" | |
| state.status = StateStatusEnum.EXECUTED | |
| state.inputs = {"key": "value"} | |
| state.outputs = {"result": "success"} | |
| state.error = "Original error" | |
| state.parents = {"parent1": PydanticObjectId()} | |
| state.does_unites = False | |
| state.retry_count = 0 | |
| state.save = AsyncMock() | |
| return state |
🤖 Prompt for AI Agents
In state-manager/tests/unit/controller/test_manual_retry_state.py around lines
33 to 49, the mocked original state is missing the retry_count attribute so
incrementing it in the controller causes a TypeError; update the fixture to set
state.retry_count to an integer (e.g., 0) on the MagicMock so
controller.increment operations work without error.
| async def test_manual_retry_state_success( | ||
| self, | ||
| mock_state_class, | ||
| mock_namespace, | ||
| mock_state_id, | ||
| mock_manual_retry_request, | ||
| mock_original_state, | ||
| mock_retry_state, | ||
| mock_request_id | ||
| ): | ||
| """Test successful manual retry state creation""" | ||
| # Arrange | ||
| mock_state_class.find_one = AsyncMock(return_value=mock_original_state) | ||
| mock_state_class.return_value = mock_retry_state | ||
|
|
||
| # Act | ||
| result = await manual_retry_state( | ||
| mock_namespace, | ||
| mock_state_id, | ||
| mock_manual_retry_request, | ||
| mock_request_id | ||
| ) | ||
|
|
||
| # Assert | ||
| assert isinstance(result, ManualRetryResponseModel) | ||
| assert result.id == str(mock_retry_state.id) | ||
| assert result.status == StateStatusEnum.CREATED | ||
|
|
||
| # Verify State.find_one was called with correct parameters | ||
| mock_state_class.find_one.assert_called_once() | ||
| call_args = mock_state_class.find_one.call_args[0] | ||
| # Check that both conditions were passed | ||
| assert len(call_args) == 2 | ||
|
|
||
| # Verify original state was updated to RETRY_CREATED | ||
| assert mock_original_state.status == StateStatusEnum.RETRY_CREATED | ||
| mock_original_state.save.assert_called_once() | ||
|
|
||
| # Verify retry state was created with correct attributes | ||
| mock_state_class.assert_called_once() | ||
| retry_state_args = mock_state_class.call_args[1] | ||
| assert retry_state_args['node_name'] == mock_original_state.node_name | ||
| assert retry_state_args['namespace_name'] == mock_original_state.namespace_name | ||
| assert retry_state_args['identifier'] == mock_original_state.identifier | ||
| assert retry_state_args['graph_name'] == mock_original_state.graph_name | ||
| assert retry_state_args['run_id'] == mock_original_state.run_id | ||
| assert retry_state_args['status'] == StateStatusEnum.CREATED | ||
| assert retry_state_args['inputs'] == mock_original_state.inputs | ||
| assert retry_state_args['outputs'] == {} | ||
| assert retry_state_args['error'] is None | ||
| assert retry_state_args['parents'] == mock_original_state.parents | ||
| assert retry_state_args['does_unites'] == mock_original_state.does_unites | ||
| assert retry_state_args['fanout_id'] == mock_manual_retry_request.fanout_id | ||
|
|
||
| # Verify retry state was inserted | ||
| mock_retry_state.insert.assert_called_once() | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick
Optionally add retry_count assertion in the success test.
Lightweight extra check that new state carries incremented retry_count.
Apply:
assert retry_state_args['does_unites'] == mock_original_state.does_unites
assert retry_state_args['fanout_id'] == mock_manual_retry_request.fanout_id
+ assert retry_state_args['retry_count'] == getattr(mock_original_state, "retry_count", 0) + 1📝 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.
| async def test_manual_retry_state_success( | |
| self, | |
| mock_state_class, | |
| mock_namespace, | |
| mock_state_id, | |
| mock_manual_retry_request, | |
| mock_original_state, | |
| mock_retry_state, | |
| mock_request_id | |
| ): | |
| """Test successful manual retry state creation""" | |
| # Arrange | |
| mock_state_class.find_one = AsyncMock(return_value=mock_original_state) | |
| mock_state_class.return_value = mock_retry_state | |
| # Act | |
| result = await manual_retry_state( | |
| mock_namespace, | |
| mock_state_id, | |
| mock_manual_retry_request, | |
| mock_request_id | |
| ) | |
| # Assert | |
| assert isinstance(result, ManualRetryResponseModel) | |
| assert result.id == str(mock_retry_state.id) | |
| assert result.status == StateStatusEnum.CREATED | |
| # Verify State.find_one was called with correct parameters | |
| mock_state_class.find_one.assert_called_once() | |
| call_args = mock_state_class.find_one.call_args[0] | |
| # Check that both conditions were passed | |
| assert len(call_args) == 2 | |
| # Verify original state was updated to RETRY_CREATED | |
| assert mock_original_state.status == StateStatusEnum.RETRY_CREATED | |
| mock_original_state.save.assert_called_once() | |
| # Verify retry state was created with correct attributes | |
| mock_state_class.assert_called_once() | |
| retry_state_args = mock_state_class.call_args[1] | |
| assert retry_state_args['node_name'] == mock_original_state.node_name | |
| assert retry_state_args['namespace_name'] == mock_original_state.namespace_name | |
| assert retry_state_args['identifier'] == mock_original_state.identifier | |
| assert retry_state_args['graph_name'] == mock_original_state.graph_name | |
| assert retry_state_args['run_id'] == mock_original_state.run_id | |
| assert retry_state_args['status'] == StateStatusEnum.CREATED | |
| assert retry_state_args['inputs'] == mock_original_state.inputs | |
| assert retry_state_args['outputs'] == {} | |
| assert retry_state_args['error'] is None | |
| assert retry_state_args['parents'] == mock_original_state.parents | |
| assert retry_state_args['does_unites'] == mock_original_state.does_unites | |
| assert retry_state_args['fanout_id'] == mock_manual_retry_request.fanout_id | |
| # Verify retry state was inserted | |
| mock_retry_state.insert.assert_called_once() | |
| async def test_manual_retry_state_success( | |
| self, | |
| mock_state_class, | |
| mock_namespace, | |
| mock_state_id, | |
| mock_manual_retry_request, | |
| mock_original_state, | |
| mock_retry_state, | |
| mock_request_id | |
| ): | |
| """Test successful manual retry state creation""" | |
| # Arrange | |
| mock_state_class.find_one = AsyncMock(return_value=mock_original_state) | |
| mock_state_class.return_value = mock_retry_state | |
| # Act | |
| result = await manual_retry_state( | |
| mock_namespace, | |
| mock_state_id, | |
| mock_manual_retry_request, | |
| mock_request_id | |
| ) | |
| # Assert | |
| assert isinstance(result, ManualRetryResponseModel) | |
| assert result.id == str(mock_retry_state.id) | |
| assert result.status == StateStatusEnum.CREATED | |
| # Verify State.find_one was called with correct parameters | |
| mock_state_class.find_one.assert_called_once() | |
| call_args = mock_state_class.find_one.call_args[0] | |
| # Check that both conditions were passed | |
| assert len(call_args) == 2 | |
| # Verify original state was updated to RETRY_CREATED | |
| assert mock_original_state.status == StateStatusEnum.RETRY_CREATED | |
| mock_original_state.save.assert_called_once() | |
| # Verify retry state was created with correct attributes | |
| mock_state_class.assert_called_once() | |
| retry_state_args = mock_state_class.call_args[1] | |
| assert retry_state_args['node_name'] == mock_original_state.node_name | |
| assert retry_state_args['namespace_name'] == mock_original_state.namespace_name | |
| assert retry_state_args['identifier'] == mock_original_state.identifier | |
| assert retry_state_args['graph_name'] == mock_original_state.graph_name | |
| assert retry_state_args['run_id'] == mock_original_state.run_id | |
| assert retry_state_args['status'] == StateStatusEnum.CREATED | |
| assert retry_state_args['inputs'] == mock_original_state.inputs | |
| assert retry_state_args['outputs'] == {} | |
| assert retry_state_args['error'] is None | |
| assert retry_state_args['parents'] == mock_original_state.parents | |
| assert retry_state_args['does_unites'] == mock_original_state.does_unites | |
| assert retry_state_args['fanout_id'] == mock_manual_retry_request.fanout_id | |
| assert retry_state_args['retry_count'] == getattr(mock_original_state, "retry_count", 0) + 1 | |
| # Verify retry state was inserted | |
| mock_retry_state.insert.assert_called_once() |
🤖 Prompt for AI Agents
In state-manager/tests/unit/controller/test_manual_retry_state.py around lines
60 to 116, add an assertion in the success test to verify the new retry state's
retry_count is incremented: after gathering retry_state_args from
mock_state_class.call_args[1], assert that retry_state_args['retry_count']
equals mock_original_state.retry_count + 1 (handle the case original retry_count
may be None by treating it as 0 in the expectation if needed).
| complex_state = MagicMock() | ||
| complex_state.id = PydanticObjectId() | ||
| complex_state.node_name = "complex_node" | ||
| complex_state.namespace_name = "test_namespace" | ||
| complex_state.identifier = "complex_identifier" | ||
| complex_state.graph_name = "complex_graph" | ||
| complex_state.run_id = "complex_run_id" | ||
| complex_state.status = StateStatusEnum.ERRORED | ||
| complex_state.inputs = { | ||
| "nested_data": {"key1": "value1", "key2": [1, 2, 3]}, | ||
| "simple_value": "test", | ||
| "number": 42 | ||
| } | ||
| complex_state.outputs = {"previous_result": "some_output"} | ||
| complex_state.error = "Complex error message" | ||
| complex_state.parents = { | ||
| "parent1": PydanticObjectId(), | ||
| "parent2": PydanticObjectId(), | ||
| "parent3": PydanticObjectId() | ||
| } | ||
| complex_state.does_unites = True | ||
| complex_state.save = AsyncMock() | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Also set retry_count on complex_state.
Keeps tests compatible with retry_count increment.
Apply:
complex_state.parents = {
"parent1": PydanticObjectId(),
"parent2": PydanticObjectId(),
"parent3": PydanticObjectId()
}
complex_state.does_unites = True
+ complex_state.retry_count = 0
complex_state.save = AsyncMock()📝 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.
| complex_state = MagicMock() | |
| complex_state.id = PydanticObjectId() | |
| complex_state.node_name = "complex_node" | |
| complex_state.namespace_name = "test_namespace" | |
| complex_state.identifier = "complex_identifier" | |
| complex_state.graph_name = "complex_graph" | |
| complex_state.run_id = "complex_run_id" | |
| complex_state.status = StateStatusEnum.ERRORED | |
| complex_state.inputs = { | |
| "nested_data": {"key1": "value1", "key2": [1, 2, 3]}, | |
| "simple_value": "test", | |
| "number": 42 | |
| } | |
| complex_state.outputs = {"previous_result": "some_output"} | |
| complex_state.error = "Complex error message" | |
| complex_state.parents = { | |
| "parent1": PydanticObjectId(), | |
| "parent2": PydanticObjectId(), | |
| "parent3": PydanticObjectId() | |
| } | |
| complex_state.does_unites = True | |
| complex_state.save = AsyncMock() | |
| complex_state = MagicMock() | |
| complex_state.id = PydanticObjectId() | |
| complex_state.node_name = "complex_node" | |
| complex_state.namespace_name = "test_namespace" | |
| complex_state.identifier = "complex_identifier" | |
| complex_state.graph_name = "complex_graph" | |
| complex_state.run_id = "complex_run_id" | |
| complex_state.status = StateStatusEnum.ERRORED | |
| complex_state.inputs = { | |
| "nested_data": {"key1": "value1", "key2": [1, 2, 3]}, | |
| "simple_value": "test", | |
| "number": 42 | |
| } | |
| complex_state.outputs = {"previous_result": "some_output"} | |
| complex_state.error = "Complex error message" | |
| complex_state.parents = { | |
| "parent1": PydanticObjectId(), | |
| "parent2": PydanticObjectId(), | |
| "parent3": PydanticObjectId() | |
| } | |
| complex_state.does_unites = True | |
| complex_state.retry_count = 0 | |
| complex_state.save = AsyncMock() |
🤖 Prompt for AI Agents
In state-manager/tests/unit/controller/test_manual_retry_state.py around lines
221 to 243, the MagicMock complex_state is missing a retry_count attribute which
breaks tests that expect retry_count to be present and incremented; add a line
setting complex_state.retry_count = 0 (or the appropriate initial integer) after
constructing complex_state so the test can increment and assert retry_count
correctly.
| empty_state = MagicMock() | ||
| empty_state.id = PydanticObjectId() | ||
| empty_state.node_name = "empty_node" | ||
| empty_state.namespace_name = "test_namespace" | ||
| empty_state.identifier = "empty_identifier" | ||
| empty_state.graph_name = "empty_graph" | ||
| empty_state.run_id = "empty_run_id" | ||
| empty_state.status = StateStatusEnum.EXECUTED | ||
| empty_state.inputs = {} | ||
| empty_state.outputs = {} | ||
| empty_state.error = None | ||
| empty_state.parents = {} | ||
| empty_state.does_unites = False | ||
| empty_state.save = AsyncMock() | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Also set retry_count on empty_state.
Apply:
empty_state.parents = {}
empty_state.does_unites = False
+ empty_state.retry_count = 0
empty_state.save = AsyncMock()📝 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.
| empty_state = MagicMock() | |
| empty_state.id = PydanticObjectId() | |
| empty_state.node_name = "empty_node" | |
| empty_state.namespace_name = "test_namespace" | |
| empty_state.identifier = "empty_identifier" | |
| empty_state.graph_name = "empty_graph" | |
| empty_state.run_id = "empty_run_id" | |
| empty_state.status = StateStatusEnum.EXECUTED | |
| empty_state.inputs = {} | |
| empty_state.outputs = {} | |
| empty_state.error = None | |
| empty_state.parents = {} | |
| empty_state.does_unites = False | |
| empty_state.save = AsyncMock() | |
| empty_state = MagicMock() | |
| empty_state.id = PydanticObjectId() | |
| empty_state.node_name = "empty_node" | |
| empty_state.namespace_name = "test_namespace" | |
| empty_state.identifier = "empty_identifier" | |
| empty_state.graph_name = "empty_graph" | |
| empty_state.run_id = "empty_run_id" | |
| empty_state.status = StateStatusEnum.EXECUTED | |
| empty_state.inputs = {} | |
| empty_state.outputs = {} | |
| empty_state.error = None | |
| empty_state.parents = {} | |
| empty_state.does_unites = False | |
| empty_state.retry_count = 0 | |
| empty_state.save = AsyncMock() |
🤖 Prompt for AI Agents
In state-manager/tests/unit/controller/test_manual_retry_state.py around lines
358 to 372, the test creates an empty_state MagicMock but omits setting its
retry_count attribute; add setting empty_state.retry_count = 0 (or the expected
default retry count for this test) so the mock matches the real object's shape
and the test assertions involving retry_count will behave correctly.
| """Test that all relevant fields from original state are preserved in retry state""" | ||
| # Arrange | ||
| original_state = MagicMock() | ||
| original_state.id = PydanticObjectId() | ||
| original_state.node_name = "preserve_test_node" | ||
| original_state.namespace_name = "preserve_test_namespace" | ||
| original_state.identifier = "preserve_test_identifier" | ||
| original_state.graph_name = "preserve_test_graph" | ||
| original_state.run_id = "preserve_test_run_id" | ||
| original_state.status = StateStatusEnum.EXECUTED | ||
| original_state.inputs = {"preserve": "input_data"} | ||
| original_state.outputs = {"should_be": "reset"} | ||
| original_state.error = "should_be_reset" | ||
| original_state.parents = {"preserve_parent": PydanticObjectId()} | ||
| original_state.does_unites = True | ||
| original_state.save = AsyncMock() | ||
|
|
||
| mock_state_class.find_one = AsyncMock(return_value=original_state) | ||
| mock_state_class.return_value = mock_retry_state | ||
|
|
||
| # Act | ||
| await manual_retry_state( | ||
| mock_namespace, | ||
| mock_state_id, | ||
| mock_manual_retry_request, | ||
| mock_request_id | ||
| ) | ||
|
|
||
| # Assert - verify all fields are correctly set | ||
| retry_state_args = mock_state_class.call_args[1] | ||
|
|
||
| # Fields that should be preserved | ||
| assert retry_state_args['node_name'] == original_state.node_name | ||
| assert retry_state_args['namespace_name'] == original_state.namespace_name | ||
| assert retry_state_args['identifier'] == original_state.identifier | ||
| assert retry_state_args['graph_name'] == original_state.graph_name | ||
| assert retry_state_args['run_id'] == original_state.run_id | ||
| assert retry_state_args['inputs'] == original_state.inputs | ||
| assert retry_state_args['parents'] == original_state.parents | ||
| assert retry_state_args['does_unites'] == original_state.does_unites | ||
| assert retry_state_args['fanout_id'] == mock_manual_retry_request.fanout_id | ||
|
|
||
| # Fields that should be reset/set to specific values | ||
| assert retry_state_args['status'] == StateStatusEnum.CREATED | ||
| assert retry_state_args['outputs'] == {} | ||
| assert retry_state_args['error'] is None | ||
|
|
||
| @patch('app.controller.manual_retry_state.logger') |
There was a problem hiding this comment.
🧹 Nitpick
Assert retry_count bump in preserved-fields test.
Verifies the incremented value on the constructed retry state args.
Apply:
assert retry_state_args['fanout_id'] == mock_manual_retry_request.fanout_id
+ # retry_count should be incremented on the new retry state
+ assert retry_state_args['retry_count'] == getattr(original_state, "retry_count", 0) + 1Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In state-manager/tests/unit/controller/test_manual_retry_state.py around lines
432 to 479: add an assertion that the constructed retry state's retry_count is
the original state's retry_count + 1; ensure the test sets
original_state.retry_count (e.g., 0 or 1) before calling manual_retry_state if
it isn't already set, then after the call assert retry_state_args['retry_count']
== original_state.retry_count + 1 to verify the bump.
1 participant
This feature improves the system's ability to manage state retries effectively.