Skip to content

Improve error handling and type hints in session_search_tool#261

Merged
teknium1 merged 1 commit into
NousResearch:mainfrom
aydnOktay:improve/session-search-error-handling
Mar 5, 2026
Merged

Improve error handling and type hints in session_search_tool#261
teknium1 merged 1 commit into
NousResearch:mainfrom
aydnOktay:improve/session-search-error-handling

Conversation

@aydnOktay

Copy link
Copy Markdown
Contributor

This PR improves code quality in session_search_tool.py by adding comprehensive type hints to _format_timestamp and _resolve_to_parent functions, enhancing error handling with specific exception types (ValueError, OSError, OverflowError) instead of generic Exception catches, adding timeout handling for session summarization operations with a 60-second limit, improving error handling in _resolve_to_parent for database access failures with debug logging, and enhancing docstrings with Args and Returns sections. The changes improve maintainability, debugging capabilities, and prevent silent failures while maintaining backward compatibility

@teknium1 teknium1 merged commit ae3deff into NousResearch:main Mar 5, 2026
teknium1 added a commit that referenced this pull request Mar 5, 2026
Follow-up to PR #261 merge:
- Fix Optional[Any] → Union[int, float, str, None] (actually meaningful)
- Fix _resolve_to_parent return type to str (never returns None in practice)
- Trim verbose docstrings on internal helpers to single-line style
- Correct docstring that claimed 'unknown' on failure (returns str(ts))
@teknium1

teknium1 commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

Merged in the merge commit + cleaned up a few things on top in 141b12b:

  • Fixed Optional[Any] type hint → Union[int, float, str, None] (actually descriptive)
  • Corrected _resolve_to_parent return type to str (never returns None in practice)
  • Trimmed verbose Args/Returns docstrings on internal helpers to concise single-line style
  • Fixed docstring that said "unknown on failure" (function returns str(ts), not "unknown")

Thanks for the contribution, @aydnOktay — the TimeoutError handling was a genuine bug fix!

angelburgosrosado pushed a commit to angelburgosrosado/hermes-agent that referenced this pull request Apr 27, 2026
…ession_search_tool

Authored by aydnOktay. Adds TimeoutError handling for session summarization,
better exception specificity in _format_timestamp, defensive try/except in
_resolve_to_parent, and type hints.
angelburgosrosado pushed a commit to angelburgosrosado/hermes-agent that referenced this pull request Apr 27, 2026
Follow-up to PR NousResearch#261 merge:
- Fix Optional[Any] → Union[int, float, str, None] (actually meaningful)
- Fix _resolve_to_parent return type to str (never returns None in practice)
- Trim verbose docstrings on internal helpers to single-line style
- Correct docstring that claimed 'unknown' on failure (returns str(ts))
02356abc pushed a commit to 02356abc/hermes-agent that referenced this pull request May 14, 2026
…ession_search_tool

Authored by aydnOktay. Adds TimeoutError handling for session summarization,
better exception specificity in _format_timestamp, defensive try/except in
_resolve_to_parent, and type hints.
02356abc pushed a commit to 02356abc/hermes-agent that referenced this pull request May 14, 2026
Follow-up to PR NousResearch#261 merge:
- Fix Optional[Any] → Union[int, float, str, None] (actually meaningful)
- Fix _resolve_to_parent return type to str (never returns None in practice)
- Trim verbose docstrings on internal helpers to single-line style
- Correct docstring that claimed 'unknown' on failure (returns str(ts))
olympus-terminal pushed a commit to olympus-terminal/hermes-agent that referenced this pull request May 16, 2026
…ession_search_tool

Authored by aydnOktay. Adds TimeoutError handling for session summarization,
better exception specificity in _format_timestamp, defensive try/except in
_resolve_to_parent, and type hints.
olympus-terminal pushed a commit to olympus-terminal/hermes-agent that referenced this pull request May 16, 2026
Follow-up to PR NousResearch#261 merge:
- Fix Optional[Any] → Union[int, float, str, None] (actually meaningful)
- Fix _resolve_to_parent return type to str (never returns None in practice)
- Trim verbose docstrings on internal helpers to single-line style
- Correct docstring that claimed 'unknown' on failure (returns str(ts))
PowerCreek added a commit to TechDevGroup/hermes-agent that referenced this pull request May 26, 2026
…) (#113)

Wraps devagentic's ``executeWorkflowPipeline`` (NousResearch#258) +
``writeWorkflowPipeline`` (NousResearch#261) mutations as MCP tools so workers
can compose + execute pipelines directly through their tool
surface. R12 of devagentic#210 (flow-aware router umbrella).

## Tools registered

| Tool | Wraps | Returns |
|---|---|---|
| ``run_pipeline(pipeline_id, user_id)`` | ``executeWorkflowPipeline`` | ``PipelineRun {runId, status, nodeOutcomes, ...}`` |
| ``propose_pipeline(name, intent, version, nodes, edges, user_id, lineage_ref?, produced_by?)`` | ``writeWorkflowPipeline`` | ``Doc {id, content, tags, source, ts}`` |

Composes naturally: ``propose_pipeline → doc_id → run_pipeline``.

## Devagentic-side resolver verification

Schemas read directly from devagentic ``api.py`` via gh api:

- ``executeWorkflowPipeline(pipelineId: String!, userId: String!) -> PipelineRun`` (api.py:21628)
- ``writeWorkflowPipeline(name, intent, version, nodes, edges, userId, lineageRef?, producedBy?) -> Doc`` (api.py:21576)

Arg-name camelCase conversion (``pipeline_id`` → ``pipelineId``,
``lineage_ref`` → ``lineageRef``, etc.) matches strawberry's default.

Note: #100's body referenced ``ShadowNodeRef`` as the propose_pipeline
return — the actual mutation returns ``Doc``. Implementing the real
shape.

## Tests

- 10 new client-level tests in
  ``tests/plugins/devagentic-mutations/test_client.py``:
  * ``run_pipeline``: empty-args fail-soft (1), happy path with arg
    threading + nested nodeOutcomes (1), non-dict fail-soft (1).
  * ``propose_pipeline``: empty-args fail-soft (3),
    invalid-version-type fail-soft (3 parametrized), invalid
    nodes/edges-type fail-soft (2), happy path with all args
    threaded (1), optional-args-null-when-omitted (1), non-dict
    fail-soft (1).
- Contract test in
  ``tests/test_mcp_defensive_registrars.py`` extended:
  ``run_pipeline`` + ``propose_pipeline`` added to the required-G2/R12
  tool set (now 17 named devagentic tools).
- 184 total green across affected suites — no regression.

## Composition

Unblocks the worker-side pipeline-economy use case in
devagentic#210 R12. Workers can now author + execute pipelines
without shelling out to ``python api.py``. The recent #99 / #110
fixes (PRs #108 + #111) ensure tool_calls with empty content +
populated tool_calls on Groq/strict-validator paths don't cascade
through the empty-retry loop, so R12 tools are usable from
client-tier dispatches.
Egavasyug pushed a commit to Egavasyug/hermes-agent that referenced this pull request Jun 10, 2026
…ession_search_tool

Authored by aydnOktay. Adds TimeoutError handling for session summarization,
better exception specificity in _format_timestamp, defensive try/except in
_resolve_to_parent, and type hints.
Egavasyug pushed a commit to Egavasyug/hermes-agent that referenced this pull request Jun 10, 2026
Follow-up to PR NousResearch#261 merge:
- Fix Optional[Any] → Union[int, float, str, None] (actually meaningful)
- Fix _resolve_to_parent return type to str (never returns None in practice)
- Trim verbose docstrings on internal helpers to single-line style
- Correct docstring that claimed 'unknown' on failure (returns str(ts))
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