Refactor code based

[leszek3737]

Summary by Sourcery

Refactor peer, session, and conclusion handling to reuse shared helpers, strengthen validation, and move ad‑hoc JSON construction to typed request structures.

New Features:

• Add validation helpers for search parameters, session creation IDs, and dialectic options.
• Introduce reusable builders and helpers for constructing peer context messages, upload form fields, and conclusion filter payloads.
• Add typed conclusion filters structure for list and query operations.

Bug Fixes:

• Ensure conclusion list and query requests use the typed API models for request bodies, aligning tests and payload structure with the API expectations.

Enhancements:

• Refactor peer cache refresh and configuration retrieval into a shared helper method.
• Simplify peer context creation by delegating to the existing ContextBuilder API.
• Centralize search parameter validation and pagination error messages for consistency.
• Refactor blocking upload builder to delegate configuration to the async upload builder and extract upload field serialization into a reusable helper.
• Unify context system message construction for different provider formats via shared helpers.
• Simplify peer configuration mapping from JSON maps by avoiding intermediate serialization steps.

[sourcery-ai]

Reviewer's Guide

Refactors peer/session/conclusion handling to centralize validation and serialization logic, introduce reusable helpers/builders, and reduce duplication while preserving API behavior.

Sequence diagram for peer refresh and configuration retrieval via fetch_and_update_cache

sequenceDiagram
    participant Peer
    participant Routes as routes
    participant Http as HttpClient
    participant Cache as PeerCache

    Peer->>Peer: refresh()
    activate Peer
    Peer->>Peer: fetch_and_update_cache()
    Peer->>Routes: peers(workspace_id)
    Routes-->>Peer: peer_route
    Peer->>Http: post(peer_route, body_with_id, [])
    Http-->>Peer: PeerResponse
    Peer->>Cache: update metadata, configuration
    Peer-->>Peer: PeerResponse
    deactivate Peer

    Peer->>Peer: get_configuration_raw()
    activate Peer
    Peer->>Peer: fetch_and_update_cache()
    Peer-->>Peer: PeerResponse
    Peer-->>Peer: configuration (HashMap)
    deactivate Peer

Loading

File-Level Changes

Change	Details	Files
Refactor peer cache refresh and configuration retrieval via a shared helper.	Extracted HTTP fetch + cache update logic from refresh and get_configuration_raw into new fetch_and_update_cache returning PeerResponse Updated refresh to reuse fetch_and_update_cache and simply await then return Ok(()) Changed get_configuration_raw to call fetch_and_update_cache and return the configuration from the PeerResponse	src/peer.rs
Unify peer context construction and parameter validation.	Replaced manual query parameter assembly in Peer::get_context with use of ContextBuilder via context_builder() and chained setters Deduplicated search parameter validation by moving range checks from RepresentationBuilder::send and ContextBuilder::send into a shared validate_search_params helper Simplified ContextBuilder::send by introducing a push_param! macro to construct query params vector and reusing validate_search_params	src/peer.rs
Improve PeerConfig mapping implementation.	Reimplemented map_to_peer_config to explicitly clone the HashMap into a serde_json::Map and build a Value::Object instead of round-tripping through to_value Adjusted error handling to still wrap serde_json::from_value errors in HonchoError::Configuration	src/peer.rs
Add typed validation and formatting helpers for sessions and context messages.	Added SessionCreate::validate to enforce non-empty, ASCII-alphanumeric/[-_] session IDs Refactored SessionContext OpenAI/Anthropic message conversion by extracting format_peer_card and build_context_messages helpers and using them to generate tagged system/user messages in a loop	src/types/session.rs
Refactor upload file builder serialization for reuse and to support blocking API wrapper.	Introduced serialize_upload_fields helper to pre-serialize metadata/configuration/created_at from UploadFileBuilder and return a closure that adds them to a multipart Form Updated UploadFileBuilder::send to use serialize_upload_fields and removed inline serialization/closure creation Simplified BlockingUploadFileBuilder setters (peer, metadata, configuration, created_at) via a new with_inner helper that applies a transformation to the inner async UploadFileBuilder	src/session.rs src/blocking/session.rs
Introduce typed conclusion filters and switch conclusion list/query builders to use them.	Added ConclusionFilters struct with builder and optional observer_id/observed_id/session_id fields Changed ConclusionGet and ConclusionQuery types to use Option instead of generic JsonValue filters Updated ListConclusionsBuilder::send and QueryConclusionsBuilder::send to construct ConclusionGet/ConclusionQuery via their builders and serialize with serde_json::to_value, replacing manual JSON assembly Fixed tests to match new serialized structure, including omitting defaulted top_k where appropriate	src/conclusion.rs src/types/conclusion.rs
Add helper methods and docs for DialecticOptions and pagination validation.	Documented DialecticOptions::validate usage pattern as a separate step from builder finish due to bon::Builder limitations Simplified validate_pagination error string construction by using into() instead of to_string	src/types/dialectic.rs src/types/pagination.rs
Minor internal commentary and perf notes.	Documented workspace_id cloning behavior and potential Arc optimization in SessionInner	src/session.rs

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[gemini-code-assist]

Code Review

This pull request refactors several builders and structures to improve code reuse, type safety, and validation. Key changes include introducing typed conclusion filters and queries, consolidating search parameter validation, and deduplicating context message building in SessionContext. The reviewer feedback focuses on performance optimizations to eliminate unnecessary string allocations and clones, specifically in ContextBuilder::send, PeerContextOptions handling, and build_context_messages.

[sourcery-ai]

Hey - I've left some high level feedback:

• The new SessionCreate::validate helper is not wired into any call sites; consider invoking it in the session creation path (e.g., builder or constructor) so the ID constraints are actually enforced rather than relying on callers to remember to call it.
• In ListConclusionsBuilder::send and QueryConclusionsBuilder::send, request-serialization failures are mapped to HonchoError::Decode, which typically describes response-decoding issues; using a more specific or configuration/serialization-oriented error variant would make debugging failures clearer.
• The push_param! macro in ContextBuilder::send blindly calls to_string() on all option values (including String), which works today but may be surprising and brittle if additional non-Display types are added; consider constraining it to impl Display and/or keeping per-field helpers for better type safety and intent clarity.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The new `SessionCreate::validate` helper is not wired into any call sites; consider invoking it in the session creation path (e.g., builder or constructor) so the ID constraints are actually enforced rather than relying on callers to remember to call it.
- In `ListConclusionsBuilder::send` and `QueryConclusionsBuilder::send`, request-serialization failures are mapped to `HonchoError::Decode`, which typically describes response-decoding issues; using a more specific or configuration/serialization-oriented error variant would make debugging failures clearer.
- The `push_param!` macro in `ContextBuilder::send` blindly calls `to_string()` on all option values (including `String`), which works today but may be surprising and brittle if additional non-`Display` types are added; consider constraining it to `impl Display` and/or keeping per-field helpers for better type safety and intent clarity.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[greptile-apps]

Greptile Summary

This PR refactors several modules in the Honcho Rust SDK to reduce code duplication, extract shared helpers, and replace ad-hoc serde_json::json!({}) construction with typed builder structs.

• fetch_and_update_cache is extracted in peer.rs to eliminate duplicated HTTP + cache-update logic between refresh and get_configuration_raw; a validate_search_params helper deduplicates the three range-checks previously repeated in RepresentationBuilder and ContextBuilder.
• serialize_upload_fields is extracted in session.rs to isolate multipart form serialization; SessionCreate::validate() is added as a new ID-constraint checker; ConclusionFilters, ConclusionGet, and ConclusionQuery replace raw serde_json::json! bodies with typed, builder-pattern structs.
• build_context_messages / format_peer_card are extracted in types/session.rs to remove identical loops repeated in to_openai and to_anthropic.

Confidence Score: 4/5

Safe to merge; the refactoring is mechanically correct, but the new SessionCreate::validate() method is never invoked so its ID-character constraints offer no runtime protection.

All the deduplication — fetch_and_update_cache, validate_search_params, push_param!, serialize_upload_fields, build_context_messages — is functionally equivalent to the code it replaces. The one present defect is that SessionCreate::validate() is dead on arrival: it enforces a non-empty, alphanumeric-only session ID, but none of the call sites invoke it, so a caller can silently pass a malformed ID to the server.

src/types/session.rs (validate() not wired up) and src/session.rs (error-reporting order changed by serialize_upload_fields moving before required-field checks)

Important Files Changed

Filename	Overview
src/peer.rs	Extracts fetch_and_update_cache, validate_search_params, and push_param! macro to reduce duplication; the macro correctly handles bool/String fields with to_string(); map_to_peer_config simplified; all changes are functionally equivalent to the original.
src/session.rs	Extracts serialize_upload_fields before peer_id/source validation, reversing the previous error-reporting order; the 'static closure bound is correct because it captures owned data, not references.
src/types/conclusion.rs	Introduces ConclusionFilters typed struct replacing raw JSON; all fields are Option with skip-if-none, which is intentional since usage always sets observer/observed; ConclusionQuery.top_k now omits default value (10) from serialized body.
src/types/session.rs	Adds SessionCreate::validate() for ID constraints and extracts build_context_messages/format_peer_card helpers; validate() is defined but not called at any SessionCreate construction site.
src/conclusion.rs	Migrates list/query request bodies to typed builders; tests correctly updated to remove top_k: 10 from expected payloads when it matches the serialization default skip.
src/blocking/session.rs	Introduces with_inner helper to eliminate repeated Self { inner: self.inner.X(), reader_handle: self.reader_handle } boilerplate; pure style refactoring with no behavior change.
src/types/dialectic.rs	Adds documentation comment explaining why validate() is separate from the builder's build() method; doc-only change.
src/types/pagination.rs	Cosmetic: replaces .to_string() with .into() for string literals in two error messages; no behavioral change.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant Peer
    participant fetch as fetch_and_update_cache
    participant HTTP
    participant Cache as RwLock Cache

    note over Peer: refresh() — was duplicated
    Caller->>Peer: refresh()
    Peer->>fetch: fetch_and_update_cache()
    fetch->>HTTP: "POST /peers {id}"
    HTTP-->>fetch: PeerResponse
    fetch->>Cache: write metadata.clone()
    fetch->>Cache: write map_to_peer_config(configuration)
    fetch-->>Peer: Ok(PeerResponse)
    Peer-->>Caller: Ok(())

    note over Peer: get_configuration_raw() — was duplicated
    Caller->>Peer: get_configuration_raw()
    Peer->>fetch: fetch_and_update_cache()
    fetch->>HTTP: "POST /peers {id}"
    HTTP-->>fetch: PeerResponse
    fetch->>Cache: write metadata.clone()
    fetch->>Cache: write map_to_peer_config(configuration)
    fetch-->>Peer: Ok(PeerResponse)
    Peer-->>Caller: Ok(resp.configuration)

Loading

_{Reviews (1): Last reviewed commit: "refactor: address code review cleanup" | Re-trigger Greptile}