feat: Improve blocking API safety, HTTP reliability, and batch error reporting

[leszek3737]

Refactor the blocking API to use a multi-threaded runtime and return Result for safer, non-panicking error handling outside async contexts. Enhance HTTP client robustness with idempotent retry logic, jittered backoff, and correct GET requests for workspace metadata and configuration. Introduce PartialFailure error for chunked batch operations to report successfully processed items. Centralize search parameter validation and query parameter generation.

[sourcery-ai]

Sorry @leszek3737, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[gemini-code-assist]

Code Review

This pull request refactors the blocking client to return Results instead of panicking when called within an async runtime, configures a multi-threaded runtime, and restricts HTTP retries to idempotent methods. It also updates workspace queries to use GET requests instead of POST, introduces jittered backoff delays, and consolidates search parameter validation. Feedback on these changes highlights that using .ok().flatten() in the blocking iterator silently discards runtime configuration errors, and warns that the new jitter logic using entropy % 500 on sub-second nanoseconds may fail on platforms with coarse clock resolutions, suggesting an entropy scrambling step to ensure uniform distribution.

[greptile-apps]

Greptile Summary

This PR makes the blocking API non-panicking by having block_on return Result instead of panicking when called from within an async runtime, switches to a multi-thread Tokio runtime, gates HTTP retries on idempotency (GET/DELETE/PUT only, not POST/PATCH), fixes get_metadata/get_configuration to use GET instead of POST, introduces PartialFailure for mid-batch message errors, and centralises search-parameter validation and query-param building.

• Blocking API safety: block_on now returns Result<F::Output> — callers propagate the error with ? instead of panicking; BlockingIter::next() uses .ok().flatten() to satisfy Iterator, which silently converts a runtime-guard error into None.
• HTTP reliability: retry logic is now gated behind is_idempotent, adds jittered_delay to spread retry storms, and replaces Url::join with build_url to correctly handle base-URL path prefixes.
• Batch error reporting: add_messages wraps partially-completed chunk results in the new HonchoError::PartialFailure variant so callers can recover successfully-created messages before the failure point.

Confidence Score: 4/5

Safe to merge; all behavioural changes are intentional and well-tested. The three findings are non-blocking quality issues.

The core changes are solid: the blocking runtime guard, idempotent-only retry logic, correct GET for workspace reads, and PartialFailure batch reporting all work as described. The three findings are cosmetic or edge-case: the Iterator silent-None issue is a trait constraint, the search_max_distance label in a conclusion builder error message is misleading, and the jitter constant fallback on a backwards clock is a corner case on unusual hardware. None affect the happy path or data correctness.

src/blocking/iter.rs and src/conclusion.rs are worth a second look before merge.

Important Files Changed

Filename	Overview
src/blocking/runtime.rs	Switches from current-thread to multi-thread runtime; block_on now returns Result instead of panicking when called from async context. Logic is clean.
src/blocking/iter.rs	Adapts Iterator::next() to new block_on Result return; .ok().flatten() silently turns a runtime-guard error into end-of-stream None, which could mislead callers.
src/http/client.rs	Adds idempotency-gated retry logic, jittered backoff, and a correct build_url helper; jitter falls back to a deterministic constant when SystemTime is unavailable.
src/client.rs	get_metadata, get_configuration, and get_configuration_raw now use GET instead of POST get-or-create; refresh is collapsed to a single GET. Semantically correct fix.
src/error.rs	Introduces PartialFailure variant for chunked batch ops with helpers is_partial_failure and into_partial_failure; well-structured and documented.
src/session.rs	Batch add_messages now returns PartialFailure on mid-batch error; context param building extracted to to_query_params(); validate_search_params centralized.
src/conclusion.rs	Uses centralized validate_search_params; adds empty-query check. Error message for distance field says search_max_distance instead of distance.
src/types/session.rs	Extracts validate_search_params and to_query_params; removes duplicated validation code across session and conclusion modules.
src/http/sse.rs	UTF-8 safe log truncation using char_indices instead of raw byte indexing; fixes latent panic on multi-byte characters near byte 100.
src/types/pagination.rs	Adds with_fetcher on PageResponse to avoid an extra clone, plus items_ref and into_items helpers; paginate_post now avoids intermediate Page allocation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[blocking API call] --> B{block_on}
    B --> C{Handle::try_current?}
    C -- "Ok: inside async runtime" --> D["Err(Configuration)"]
    D --> E[caller gets Err propagated via ?]
    C -- "Err: no active runtime" --> F[multi-thread runtime.block_on future]
    F --> G["Ok(async result)"]
    G --> H[caller unwraps with ?]
    H --> I{HTTP request}
    I --> J{response ok?}
    J -- "yes" --> K[return result]
    J -- "no" --> L{is_idempotent AND retryable status?}
    L -- "yes" --> M[jittered_delay then retry]
    M --> I
    L -- "no POST/PATCH" --> N[return Err immediately]
    L -- "idempotent, non-retryable" --> N

    subgraph BlockingIter
        O[Iterator::next] --> P{block_on StreamNext}
        P -- "Ok(Some item)" --> Q[yield item]
        P -- "Ok(None)" --> R[end of stream]
        P -- "Err runtime guard" --> S["None - silent stop"]
    end

Loading

Comments Outside Diff (1)

1. src/blocking/iter.rs, line 45-51 (link)

   Silent stream termination on async-context violation

   .ok().flatten() converts a Configuration error ("cannot be called from within an async runtime") into None, making the iterator appear to have cleanly ended. If a caller somehow holds a BlockingIter and polls it from within an async context (e.g. after moving it across a thread boundary), every .next() will silently return None with no error surfaced. The for-loop will exit immediately and any remaining stream items will be silently dropped, indistinguishable from a clean end-of-stream.

_{Reviews (1): Last reviewed commit: "feat: Improve blocking API safety, HTTP ..." | Re-trigger Greptile}