From 4ed25cdf7a2c2ddbfade18689ff4988357273b4c Mon Sep 17 00:00:00 2001 From: Finesssee Date: Sat, 5 Sep 2026 18:07:58 +0700 Subject: [PATCH] Document dependency consolidation candidates --- ...006-dependency-consolidation-candidates.md | 245 ++++++++++++++++++ 1 file changed, 245 insertions(+) create mode 100644 docs/adr/0006-dependency-consolidation-candidates.md diff --git a/docs/adr/0006-dependency-consolidation-candidates.md b/docs/adr/0006-dependency-consolidation-candidates.md new file mode 100644 index 0000000000..df45f9f388 --- /dev/null +++ b/docs/adr/0006-dependency-consolidation-candidates.md @@ -0,0 +1,245 @@ +# ADR 0006: Dependency consolidation candidates + +Date: 2026-09-05 +Status: Proposed + +## Context + +Win-CodexBar has grown across roughly 70 providers, a Rust backend, a Tauri 2 +shell, and a React frontend. The codebase now contains several places where the +same domain information or infrastructure policy is expressed repeatedly. +Adding dependencies is only worthwhile when a crate removes measurable +boilerplate, reduces synchronization points, or materially improves +correctness. This ADR records candidates for review before implementation. It +does not approve adding every crate listed below. + +The September 2026 audit found these concrete pressure points: + +- `rust/src/core/provider.rs` is 1,131 lines. `ProviderId` has 70 variants, with + repeated handwritten tables for iteration, CLI names, display names, cookie + domains, aliases, and brand colors. The largest repeated metadata blocks span + roughly 540 lines. +- `apps/desktop-tauri/src/types/bridge.ts` is 958 lines with 101 exported + types/interfaces that must stay aligned with Rust DTOs. +- `apps/desktop-tauri/src/lib/tauri.ts` is 534 lines with 91 handwritten invoke + wrappers, while the Rust shell exposes 98 `#[tauri::command]` functions. +- `FetchContext` is manually constructed at 26 call sites. +- The Rust code contains about 20 `lock().unwrap*` sites, including eight + explicit poison-recovery patterns using `into_inner()`. +- There are many manual `sort`, `dedup`, `HashSet` uniqueness, grouping, and + collection reshaping paths across provider, session, cost, and dashboard + code. +- URL-like configuration and command inputs are commonly represented as raw + `String`/`&str`, including gateway, proxy, dashboard, status, release, and + provider endpoint URLs. `validate_external_url` currently performs manual + HTTP(S) prefix validation. +- Provider and configuration parsing has many generic Serde error mappings that + report the parser error but not the precise failing field path. +- Several in-memory caches hand-roll `Mutex>`, timestamps, and TTL + checks for provider, chart/local-usage, status, token, and pricing data. +- Cost and spend calculations are extensively represented as `f64`. + +## Decision + +Use a tiered adoption policy. A dependency is added only in the PR that also +uses it to remove or centralize existing code. Do not add dormant dependencies +for hypothetical future use. + +### Tier 1: highest-confidence candidates + +#### `strum` + +Use derives such as `EnumIter`, `EnumString`, `Display`, `VariantArray`, and +variant metadata where they eliminate repeated enum plumbing. + +Primary target: `ProviderId` and other enums that manually implement `all`, +`parse`, `as_str`, `display_name`, or alias tables. + +Acceptance criteria: + +- provider metadata has one obvious source of truth per property; +- aliases and compatibility names remain behaviorally identical; +- exhaustive provider factory behavior remains compile-time checked; +- net handwritten LOC decreases materially. + +#### `ts-rs` + +Generate TypeScript DTO definitions from Rust for bridge types that are true +wire contracts. Keep frontend-only unions/types handwritten when Rust is not +the source of truth. + +Primary target: the mirrored portions of `src/types/bridge.ts`. + +Acceptance criteria: + +- generated files are deterministic and checked in or generated by an explicit + reproducible step; +- Serde rename/tag behavior is covered by tests; +- the generated boundary removes manual Rust/TypeScript synchronization rather + than introducing a second schema layer; +- no runtime dependency is added to the frontend. + +Do not adopt `tauri-specta` in the same step unless its Tauri 2 release line is +independently proven stable and it replaces enough of `src/lib/tauri.ts` to +justify the additional integration surface. + +#### `serde_with` + +Use only where it replaces custom Serde adapters or repetitive conversion +logic, for example display/from-string bridges or optional/default handling. + +Acceptance criteria: each use deletes or simplifies existing serialization +code; avoid cosmetic attribute churn. + +#### `url` + +Promote URL values to `url::Url` at validation/security boundaries where the +value is semantically a URL. The crate is already present transitively through +`reqwest`, so making it direct should not materially enlarge the dependency +graph at the current lockfile state. + +Primary targets: external URL validation and provider/gateway/proxy endpoints. + +Acceptance criteria: + +- scheme allowlists stay explicit; +- file/javascript and other unsafe schemes remain rejected where applicable; +- persisted wire compatibility is preserved when settings currently store + strings. + +#### `parking_lot` + +Use for internal synchronization where standard-library poisoning semantics are +not part of the intended behavior and where it removes repeated poison recovery. + +Acceptance criteria: + +- no lock is held across `.await`; +- migration is focused rather than repo-wide mechanical churn; +- test-only locks do not justify the dependency by themselves. + +#### `itertools` + +Use for collection transforms that become materially clearer, especially +unique/sorted/grouped pipelines that currently require temporary sets or +multi-step mutation. + +Acceptance criteria: the replacement is shorter and easier to read than the +standard-library version; simple iterator chains remain standard library only. + +#### `bon` + +Prototype on `FetchContext`, which has 26 literal construction sites, before +using it elsewhere. + +Acceptance criteria: + +- call sites communicate only intentional overrides; +- defaults remain centralized; +- builder generation does not obscure required fields or make diagnostics + worse; +- measurable net LOC/readability improvement is shown in the prototype diff. + +#### `serde_path_to_error` + +Use at user-facing or provider-facing JSON boundaries where a precise field path +would materially improve diagnostics. + +Acceptance criteria: preserve the existing provider-specific context while +adding the failing data path; do not wrap silent best-effort parsers whose +failure is intentionally ignored. + +### Tier 2: architecture-specific candidates + +These need a focused prototype and should not be added merely because the crate +is useful in general. + +#### `tokio-util` + +Candidate for `CancellationToken` in refresh/probe/background-task ownership. +It is already present transitively through the current HTTP stack. Adopt only +where it replaces bespoke cancellation flags/channels and clarifies task +lifetime. + +#### `secrecy` + +Candidate for API keys, tokens, and other credentials that currently travel as +ordinary `String`s. Adoption must integrate with existing DPAPI, keyring, +redaction, and persistence behavior rather than creating a parallel secret +model. + +#### `moka` + +Candidate for simple in-memory TTL caches currently implemented as +`Mutex>` plus timestamps. Do not replace the JSONL/disk cost cache +or other caches with domain-specific persistence/invalidation semantics unless a +prototype proves equivalence. + +#### `reqwest-middleware` and `reqwest-retry` + +Candidate for genuinely shared idempotent HTTP retry/backoff policy. The repo is +currently on `reqwest 0.12`, so compatible middleware versions must be verified +before adoption. Provider-specific authentication refresh, WAF fallback, +alternate-host logic, and semantic retries must remain provider-owned. + +#### `rust_decimal` + +Candidate for monetary totals if floating-point rounding is demonstrated to be +a product-level correctness problem. Migration would be broad because cost, +pricing, bridge DTOs, formatting, tests, and persisted/exported values currently +use `f64`. + +Do not migrate merely for theoretical precision. First add a failing test or +real rounding example that changes an observable result. + +#### `phf` + +Candidate for immutable lookup tables that are currently runtime-built +`LazyLock>` values. Prefer ordinary `match`, slices, or arrays when +those are already clearer and compile-time cheap. + +## Rejected default: dependency accumulation + +Do not add all candidates in one dependency-only PR. That would increase build, +supply-chain, maintenance, and audit surface while proving none of the claimed +benefits. + +The preferred sequence is small implementation PRs, each with before/after +evidence: + +1. `strum` for provider/enum metadata. +2. `ts-rs` for Rust-to-TypeScript wire types. +3. `url` for URL validation boundaries. +4. `bon` prototype for `FetchContext`. +5. `serde_path_to_error` for selected provider/config diagnostics. +6. `parking_lot` and `itertools` where the diff clearly pays for them. +7. Tier 2 only after a concrete problem demonstrates the need. + +`serde_with` may be folded into one of the above only when the same PR has a +clear serialization target. + +## Review questions + +Reviewers should challenge each candidate with the same questions: + +1. Which current code is deleted or centralized by this crate? +2. How many call sites or synchronization points disappear? +3. Does the dependency become part of a public/wire/persistence contract? +4. What new failure mode, compile-time cost, runtime cost, or supply-chain + surface does it add? +5. Could a small local helper or derive already in the graph solve the same + problem more simply? +6. Is the migration independently reversible? +7. What test proves behavior stayed identical? + +## Consequences + +- Dependency additions become evidence-driven rather than preference-driven. +- The highest-value work focuses on duplicated sources of truth first: + `ProviderId` metadata and the Rust/TypeScript bridge contract. +- Large architectural migrations are split from low-risk boilerplate removal. +- Reviewers can reject individual candidates without blocking the rest of the + strategy. +- This ADR is intentionally `Proposed`; accepting it does not automatically + authorize every Tier 1 or Tier 2 dependency. Each implementation PR still + needs its own measured justification and tests.