My agentic slop goes here. Not intended for anyone else!
anil.recoil.org
1# Production Quality TODO
2
3This document tracks remaining tasks to make the immutable Requests.t and refactored library patterns production-ready.
4
5## High Priority
6
7### 1. Update Immiche README.md ⚠️
8**Status:** Outdated
9**Location:** `immiche/README.md`
10
11The README still documents the old API (passing `~sw ~clock ~net ~base_url ~api_key` on every call) instead of the new session-based pattern.
12
13**Tasks:**
14- [ ] Update API signatures to show `create` function
15- [ ] Update example code to show session creation
16- [ ] Document connection pooling benefits
17- [ ] Update "Implementation Notes" to mention `Requests.t` instead of `Requests.One`
18- [ ] Add example of sharing sessions across multiple libraries
19
20**Example of needed change:**
21```ocaml
22(* OLD - documented in README *)
23let people = Immiche.fetch_people ~sw ~clock ~net ~base_url ~api_key
24
25(* NEW - what actually works now *)
26let client = Immiche.create ~sw ~env ~base_url ~api_key () in
27let people = Immiche.fetch_people client
28```
29
30### 2. Improve Error Handling in Immiche
31**Status:** Inconsistent
32**Location:** `immiche/immiche.ml:79,92,133`
33
34Currently uses `failwith` for HTTP errors instead of returning Result types consistently.
35
36**Issues:**
37- `fetch_people`, `fetch_person`, `search_person` use `failwith` for HTTP errors
38- `download_thumbnail` correctly returns `Result`
39- Inconsistent error handling makes it hard to gracefully handle failures
40
41**Tasks:**
42- [ ] Change `fetch_people` to return `(people_response, [> `Msg of string]) result`
43- [ ] Change `fetch_person` to return `(person, [> `Msg of string]) result`
44- [ ] Change `search_person` to return `(person list, [> `Msg of string]) result`
45- [ ] Update `immiche/immiche.mli` with new signatures
46- [ ] Update `bushel/bin/bushel_faces.ml` to handle Result types
47- [ ] Document error cases in docstrings
48
49### 3. Add Type Aliases for Verbose Signatures
50**Status:** Verbose
51**Location:** `immiche/immiche.mli`, `zotero-translation/zotero_translation.mli`
52
53Type signatures like `([> float Eio.Time.clock_ty ] Eio.Resource.t, [> [> `Generic ] Eio.Net.ty ] Eio.Resource.t) t` are extremely verbose.
54
55**Tasks:**
56- [ ] Consider adding type aliases:
57 ```ocaml
58 type client =
59 ([> float Eio.Time.clock_ty ] Eio.Resource.t,
60 [> [> `Generic ] Eio.Net.ty ] Eio.Resource.t) t
61
62 val fetch_people : client -> people_response
63 ```
64- [ ] Evaluate if this improves or harms the API (might hide important type info)
65- [ ] If beneficial, apply to both Immiche and zotero-translation
66
67## Medium Priority
68
69### 4. Add Tests for New Immiche Pattern
70**Status:** No tests
71**Location:** None - needs creation
72
73The Immiche library was refactored but has no tests.
74
75**Tasks:**
76- [ ] Create `immiche/test/test_immiche.ml`
77- [ ] Add test for `create` function
78- [ ] Add tests for all API functions with mocked responses
79- [ ] Test connection pooling behavior (multiple requests reuse connections)
80- [ ] Test session sharing (derived sessions share pools)
81- [ ] Add to dune test suite
82
83### 5. Verify Zotero-translation with Immutable Requests.t
84**Status:** Unknown compatibility
85**Location:** `zotero-translation/`
86
87Zotero-translation was already using session pattern before we made Requests.t immutable. Need to verify it still works correctly.
88
89**Tasks:**
90- [ ] Review if zotero-translation needs updates for immutable Requests.t
91- [ ] Check if any derived sessions are created (and if pools are shared correctly)
92- [ ] Add tests if missing
93- [ ] Document session sharing pattern if used
94
95### 6. Document Thread Safety / Concurrency Model
96**Status:** Unclear
97**Location:** Documentation
98
99With immutable sessions that share mutable cookie_jar and statistics, the concurrency model needs documentation.
100
101**Tasks:**
102- [ ] Document that derived sessions share:
103 - Connection pools (intentional - for performance)
104 - Cookie jar (intentional - session state)
105 - Statistics (intentional - unified monitoring)
106- [ ] Document that cookie_mutex protects concurrent cookie access
107- [ ] Clarify if derived sessions can be used safely across Eio fibers
108- [ ] Add concurrency examples to documentation
109
110### 7. Enhanced API Documentation
111**Status:** Basic
112**Location:** All `.mli` files
113
114Current documentation is minimal. Could add more examples and edge case documentation.
115
116**Tasks:**
117- [ ] Add usage examples to `requests/lib/requests.mli` showing:
118 - Creating base session
119 - Deriving sessions with different configs
120 - Sharing pools across derived sessions
121- [ ] Document what happens when you derive from derived sessions
122- [ ] Add performance notes about connection pooling
123- [ ] Document cookie handling behavior
124
125## Low Priority
126
127### 8. Consider Immutable Statistics
128**Status:** Design decision
129**Location:** `requests/lib/requests.ml:51-53`
130
131Statistics (requests_made, total_time, retries_count) are still mutable and shared across derived sessions.
132
133**Trade-offs:**
134- **Current (mutable):** Simple, tracks total usage across all derived sessions
135- **Alternative (immutable):** Would require returning `(Response.t * t)` from all request functions
136
137**Tasks:**
138- [ ] Evaluate if immutable statistics would be valuable
139- [ ] If yes, design API that returns updated sessions from request functions
140- [ ] If no, document the design decision and rationale
141
142### 9. Add Examples Directory
143**Status:** No standalone examples
144**Location:** None
145
146Would be helpful to have example code showing common patterns.
147
148**Tasks:**
149- [ ] Create `examples/` directory
150- [ ] Add `examples/immiche_basic.ml` - basic Immiche usage
151- [ ] Add `examples/immiche_advanced.ml` - session sharing across APIs
152- [ ] Add `examples/requests_sessions.ml` - derived sessions with shared pools
153- [ ] Add `examples/zotero_immiche_shared.ml` - sharing pools between libraries
154- [ ] Ensure examples build and run
155
156### 10. Performance Benchmarks
157**Status:** No benchmarks
158**Location:** None
159
160Would be valuable to have benchmarks showing connection pooling benefits.
161
162**Tasks:**
163- [ ] Create benchmark comparing:
164 - Requests.One (no pooling) vs Requests.t (with pooling)
165 - Single session vs derived sessions (verify pool sharing)
166 - Sequential vs parallel requests
167- [ ] Document performance characteristics
168- [ ] Add to CI if significant
169
170## Breaking Changes Documented
171
172### Immiche API (Breaking)
173**Old API:**
174```ocaml
175val fetch_people :
176 sw:Eio.Switch.t ->
177 clock:_ Eio.Time.clock ->
178 net:_ Eio.Net.t ->
179 base_url:string ->
180 api_key:string ->
181 people_response
182```
183
184**New API:**
185```ocaml
186val create :
187 sw:Eio.Switch.t ->
188 env:< clock:_; net:_; fs:_; .. > ->
189 ?requests_session:_ Requests.t ->
190 base_url:string ->
191 api_key:string ->
192 unit -> t
193
194val fetch_people : t -> people_response
195```
196
197**Migration Required:**
198- All Immiche users must update to create a client first
199- Only known usage: `bushel/bin/bushel_faces.ml` (already updated)
200
201### Requests Configuration Functions (Breaking)
202**Old API:**
203```ocaml
204val set_auth : t -> Auth.t -> unit
205```
206
207**New API:**
208```ocaml
209val set_auth : t -> Auth.t -> t
210```
211
212**Migration Required:**
213- Must capture returned session: `let req = Requests.set_auth req auth`
214- All usages updated: ocurl.ml, test_requests.ml, jmap_client.ml
215
216## Notes
217
218- **Cookie handling:** Still mutable via shared cookie_jar - this is intentional for session state
219- **Statistics:** Still mutable and shared - tracks total usage across derived sessions
220- **Connection pools:** Shared across all derived sessions - major performance benefit
221- **No backward compatibility:** Breaking changes are acceptable for this monorepo
222
223## Completion Checklist
224
225Before considering this production-ready:
226
227- [ ] High priority items completed
228- [ ] README.md updated for Immiche
229- [ ] Error handling consistent across Immiche
230- [ ] At least basic tests added for Immiche
231- [ ] Documentation covers concurrency model
232- [ ] All examples in docs are verified to work