anil.recoil.org / slop
My agentic slop goes here. Not intended for anyone else!
fork atom
slop / stack / TODO.md
at main 7.9 kB view raw view code

Production Quality TODO#

This document tracks remaining tasks to make the immutable Requests.t and refactored library patterns production-ready.

High Priority#

1. Update Immiche README.md ⚠️#

Status: Outdated Location: immiche/README.md

The README still documents the old API (passing ~sw ~clock ~net ~base_url ~api_key on every call) instead of the new session-based pattern.

Tasks:

  • Update API signatures to show create function
  • Update example code to show session creation
  • Document connection pooling benefits
  • Update "Implementation Notes" to mention Requests.t instead of Requests.One
  • Add example of sharing sessions across multiple libraries

Example of needed change:

(* OLD - documented in README *)
let people = Immiche.fetch_people ~sw ~clock ~net ~base_url ~api_key

(* NEW - what actually works now *)
let client = Immiche.create ~sw ~env ~base_url ~api_key () in
let people = Immiche.fetch_people client

2. Improve Error Handling in Immiche#

Status: Inconsistent Location: immiche/immiche.ml:79,92,133

Currently uses failwith for HTTP errors instead of returning Result types consistently.

Issues:

  • fetch_people, fetch_person, search_person use failwith for HTTP errors
  • download_thumbnail correctly returns Result
  • Inconsistent error handling makes it hard to gracefully handle failures

Tasks:

  • Change fetch_people to return (people_response, [> Msg of string]) result`
  • Change fetch_person to return (person, [> Msg of string]) result`
  • Change search_person to return (person list, [> Msg of string]) result`
  • Update immiche/immiche.mli with new signatures
  • Update bushel/bin/bushel_faces.ml to handle Result types
  • Document error cases in docstrings

3. Add Type Aliases for Verbose Signatures#

Status: Verbose Location: immiche/immiche.mli, zotero-translation/zotero_translation.mli

Type signatures like ([> float Eio.Time.clock_ty ] Eio.Resource.t, [> [> Generic ] Eio.Net.ty ] Eio.Resource.t) t` are extremely verbose.

Tasks:

  • Consider adding type aliases: 
    type client =
  ([> float Eio.Time.clock_ty ] Eio.Resource.t,
   [> [> `Generic ] Eio.Net.ty ] Eio.Resource.t) t

val fetch_people : client -> people_response
  • Evaluate if this improves or harms the API (might hide important type info)
  • If beneficial, apply to both Immiche and zotero-translation

Medium Priority#

4. Add Tests for New Immiche Pattern#

Status: No tests Location: None - needs creation

The Immiche library was refactored but has no tests.

Tasks:

  • Create immiche/test/test_immiche.ml
  • Add test for create function
  • Add tests for all API functions with mocked responses
  • Test connection pooling behavior (multiple requests reuse connections)
  • Test session sharing (derived sessions share pools)
  • Add to dune test suite

5. Verify Zotero-translation with Immutable Requests.t#

Status: Unknown compatibility Location: zotero-translation/

Zotero-translation was already using session pattern before we made Requests.t immutable. Need to verify it still works correctly.

Tasks:

  • Review if zotero-translation needs updates for immutable Requests.t
  • Check if any derived sessions are created (and if pools are shared correctly)
  • Add tests if missing
  • Document session sharing pattern if used

6. Document Thread Safety / Concurrency Model#

Status: Unclear Location: Documentation

With immutable sessions that share mutable cookie_jar and statistics, the concurrency model needs documentation.

Tasks:

  • Document that derived sessions share:
    • Connection pools (intentional - for performance)
    • Cookie jar (intentional - session state)
    • Statistics (intentional - unified monitoring)
  • Document that cookie_mutex protects concurrent cookie access
  • Clarify if derived sessions can be used safely across Eio fibers
  • Add concurrency examples to documentation

7. Enhanced API Documentation#

Status: Basic Location: All .mli files

Current documentation is minimal. Could add more examples and edge case documentation.

Tasks:

  • Add usage examples to requests/lib/requests.mli showing:
    • Creating base session
    • Deriving sessions with different configs
    • Sharing pools across derived sessions
  • Document what happens when you derive from derived sessions
  • Add performance notes about connection pooling
  • Document cookie handling behavior

Low Priority#

8. Consider Immutable Statistics#

Status: Design decision Location: requests/lib/requests.ml:51-53

Statistics (requests_made, total_time, retries_count) are still mutable and shared across derived sessions.

Trade-offs:

  • Current (mutable): Simple, tracks total usage across all derived sessions
  • Alternative (immutable): Would require returning (Response.t * t) from all request functions

Tasks:

  • Evaluate if immutable statistics would be valuable
  • If yes, design API that returns updated sessions from request functions
  • If no, document the design decision and rationale

9. Add Examples Directory#

Status: No standalone examples Location: None

Would be helpful to have example code showing common patterns.

Tasks:

  • Create examples/ directory
  • Add examples/immiche_basic.ml - basic Immiche usage
  • Add examples/immiche_advanced.ml - session sharing across APIs
  • Add examples/requests_sessions.ml - derived sessions with shared pools
  • Add examples/zotero_immiche_shared.ml - sharing pools between libraries
  • Ensure examples build and run

10. Performance Benchmarks#

Status: No benchmarks Location: None

Would be valuable to have benchmarks showing connection pooling benefits.

Tasks:

  • Create benchmark comparing:
    • Requests.One (no pooling) vs Requests.t (with pooling)
    • Single session vs derived sessions (verify pool sharing)
    • Sequential vs parallel requests
  • Document performance characteristics
  • Add to CI if significant

Breaking Changes Documented#

Immiche API (Breaking)#

Old API:

val fetch_people :
  sw:Eio.Switch.t ->
  clock:_ Eio.Time.clock ->
  net:_ Eio.Net.t ->
  base_url:string ->
  api_key:string ->
  people_response

New API:

val create :
  sw:Eio.Switch.t ->
  env:< clock:_; net:_; fs:_; .. > ->
  ?requests_session:_ Requests.t ->
  base_url:string ->
  api_key:string ->
  unit -> t

val fetch_people : t -> people_response

Migration Required:

  • All Immiche users must update to create a client first
  • Only known usage: bushel/bin/bushel_faces.ml (already updated)

Requests Configuration Functions (Breaking)#

Old API:

val set_auth : t -> Auth.t -> unit

New API:

val set_auth : t -> Auth.t -> t

Migration Required:

  • Must capture returned session: let req = Requests.set_auth req auth
  • All usages updated: ocurl.ml, test_requests.ml, jmap_client.ml

Notes#

  • Cookie handling: Still mutable via shared cookie_jar - this is intentional for session state
  • Statistics: Still mutable and shared - tracks total usage across derived sessions
  • Connection pools: Shared across all derived sessions - major performance benefit
  • No backward compatibility: Breaking changes are acceptable for this monorepo

Completion Checklist#

Before considering this production-ready:

  • High priority items completed
  • README.md updated for Immiche
  • Error handling consistent across Immiche
  • At least basic tests added for Immiche
  • Documentation covers concurrency model
  • All examples in docs are verified to work