Menu

#454 feat(llm): centralised AI-provider selection - custom endpoints, connectivity testing, and a dev-only LLM Lab

closed
nobody
None
2026-07-18
2026-07-17
Anonymous
No

Originally created by: adityaharishch

What does this PR do?

This is the feat/llm-provider-enum integration branch. Its through-line is giving Meridian a real, user-owned AI-provider layer - one place to pick which model runs the prose pipeline, add your own cloud endpoint, prove it actually works before it's trusted in production, and (in dev) compare providers head-to-head. Alongside that it folds in two adjacent pieces of pipeline work that matured on the same branch: worklog posting to multiple tickets and the plan-task authoring flow.

It targets pre-main and carries the merged, review-fixed work from [#452], [#453], [#455] and [#457].

1. Centralised AI-provider selection

  • A single provider choice (meridian-core/src/llm_provider.rs, settings.rs) surfaced in Settings -> Intelligence and the first-run setup wizard, replacing the implicit "whatever CLI we found" behaviour. Consumed through LlmProviderPicker.tsx (restyled for readability, the "recommended" badge dropped) and resolver.rs.

2. Custom OpenAI-compatible cloud providers

  • Add and select your own OpenAI-compatible endpoint straight from the provider picker (src/llm/openai_compat.rs, custom_llm.rs, CustomProviders.tsx).
  • Before an endpoint is allowed to run production work, Meridian probes its real schema support (src/llm/probe.rs, schema.rs) and gates on it, so a model that can't honour structured output never silently corrupts the pipeline.
  • Codex now speaks OpenAI's strict-schema dialect and reports its real error (src/llm/codex.rs).

3. Provider connectivity testing

  • A real connectivity test for every installed provider CLI and custom endpoint (src/llm/detect.rs), wired into both Settings and the setup wizard, with results cached to disk.
  • Honours the provider's own rate-limit reset time rather than a fixed backoff (src/llm/reset_time.rs), and serialises cache writes so a concurrent "Rescan" can't lose results.

4. LLM Lab - dev-only comparison harness

  • A dev-channel-only modal (ui/components/timeline/llmlab/*) that replays one prose stage (hour report, workstream fold, worklog generate, day fold) across several provider/model variants and renders the outcomes as real dashboard timelines for side-by-side comparison.
  • Backed by an experiment ledger (src/llm_experiment/*, migration 064), a meridian-core reader, dev-only tray commands, and a meridian llm-experiment CLI.

5. Worklog posting to multiple tickets (migrations 061-063)

  • The worklog matcher now compares a day's work against that day's planned tasks only, with a manual target override so the user can file against any board ticket (061).
  • One day-task's update can post to several tickets, promoted into a child table with per-ticket idempotency (062) plus a pre-call post-attempt marker that closes the crash-between-post-and-record double-post gap (063). Surfaced via WorklogTargets.tsx / WorklogTicketPicker.tsx and a large rework of pm_worklog/generate.rs.

6. Plan-task authoring

  • Create / draft / edit planned tasks (src/plan_tasks/*, task_create.rs, services/prompts/plan-task-draft.md), with a task composer and drag-and-drop plan board (ui/components/plan/*, TaskComposer.tsx).

7. Pipeline correctness & dev-loop fixes (folded from [#453]/#457)

  • Geometry-independent worklog fold idempotency and bracket-gated report parsing (worklog_pipeline/).
  • coding_deadline anchored to the hour's end rather than process start.
  • dev-start.sh now watches only what rebuilds the daemon and stops a stray installed-runtime MLX server so a dev run uses its own services/.

How was it tested?

  • [x] cargo test passes (new coverage in meridian-core/tests/readers.rs, worklog/llm unit tests)
  • [x] cargo clippy -- -D warnings passes
  • [x] cargo fmt --check passes
  • [x] UI builds (cd ui && npm run build) - full pre-push suite green on the latest push
  • [ ] Manually tested the relevant flows

Notes for the reviewer

  • Large branch: ~120 files. It's an integration branch that accumulated several feature merges; the sections above are the review seams.
  • Runtime version bump required before merge: this PR changes services/, so services/pyproject.toml must be bumped above the live runtime-staging (currently 1.70.0) or the rebuilt MLX runtime won't ship to staging (the auto-upgrade is an equality check). The CI "Runtime version bump check" enforces this.
  • New migrations 061-065 are append-only. Most are additive; 062 is destructive-but-safe - it promotes the worklog match columns into the day_task_worklog_targets child table via a carry-over INSERT...SELECT and then DROP COLUMNs the six migrated fields (the data moves, nothing is lost). 065 (added in review) adds create_attempt_at, the write-ahead create-claim marker described in the worklog section.

Related

Tickets: #452
Tickets: #453
Tickets: #455
Tickets: #457

Discussion

  • Anonymous

    Anonymous - 2026-07-17

    Originally posted by: coderabbitai[bot]

    [!IMPORTANT]

    Review skipped

    Auto reviews are disabled on base/target branches other than the default branch.

    Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.


    ⚙️ Run configuration

    Configuration used: Organization UI

    Review profile: ASSERTIVE

    Plan: Pro Plus

    Run ID: e7fdb311-bb03-46b3-81c7-c9b2f9f0c0b4

    You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

    Use the checkbox below for a quick retry:
    - [ ] 🔍 Trigger review

    ✨ Finishing Touches
    🧪 Generate unit tests (beta) - [ ] Create PR with unit tests - [ ] Commit unit tests in branch `feat/llm-provider-enum`

    Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

    ❤️ Share - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

    Comment @coderabbitai help to get the list of available commands.

     
  • Anonymous

    Anonymous - 2026-07-18

    Originally posted by: Akarsh-Hegde

    Thanks for the thorough review - pushed 986a307c addressing it. Summary of what changed and what's deliberately deferred.

    High severity - all fixed

    #1 - LLM Lab CLI dev-gating. run/create/exec now refuse to run in a release binary (they drive real, metered completions through resolver::backend_for), matching draft-task and the tray's dev_only(). list/get stay open (reads only). Corrected cli.rs's false "no network, no cost" doc.

    #2 - Worklog multi-ticket idempotency.

    • upsert_draft/retarget_draft now run the parent guard-write and targets::replace in one transaction (targets::replace takes &mut SqliteConnection), so a concurrent approve() can't slip a comment-post between the guard matching and the child-table DELETE.
    • Added a create-claim CAS (begin_create/revert_create + migration 065's create_attempt_at), the create-step analog of your post_attempt_at. Two concurrent approves - or a crash between create_ticket returning and mark_created recording the key - can no longer each file a real ticket. New tests cover exclusivity, revert-frees-retry, stale-reclaim, and success-resolves.
    • One conscious divergence from post_attempt_at, documented in migration 065 and begin_create: a dangling create marker auto-reclaims past a bounded stale window rather than being a permanent dead end. A post marker can be a dead end because its target surfaces outcome_unknown in the UI; an approved row has no regenerate path and creates aren't surfaced, so a pure dead end would wedge the draft forever. The accepted cost is a minutes-wide window where a crash-after-success + re-approve could double-create - strictly better than a permanently stuck draft. Happy to instead surface it in the read struct/UI like outcome_unknown if you'd prefer to match precedent exactly.

    #3 - Plan-task "add" TOCTOU. The cap check, MAX(position), and INSERT are now a single atomic conditional INSERT (... HAVING COUNT(*) < cap ... ON CONFLICT DO NOTHING), so concurrent adds can't both pass the cap. Existing cap test still green.

    • Deferred (not closed here): the second-order race you noted - two concurrent creates in plan_tasks/create.rs each passing ensure_plan_has_room and each filing a real Jira/Linear ticket. That's a cross-process network call that can't sit in a SQLite transaction; a correct fix needs a slot-reservation design and belongs in its own PR with its own tests. The DB cap is now hard, so this can at most leave one orphaned ticket on a genuine race, never an over-full plan.

    #4 - base_url telemetry leak. add_custom_llm_provider's span now skips base_url too (a URL can carry a key in a query string). endpoint_id+vendor remain for debugging.

    #5 - Rate-limit backoff. Now keyed per backend (custom endpoints by id), stored in a map instead of one process-wide cell. Switching to a provider that isn't limited escapes the backoff immediately - which also dissolves the cross-process problem: the daemon holds the backoff and the tray writes settings, so the documented clear-on-write could never have fired. Dropped the misleading clear_backoff public API (now test-only). New test asserts a Claude backoff doesn't divert a switched-to Codex.

    Medium / nits - fixed

    • #6 custom_llm add/probe/remove serialize their settings.json read-modify-write behind a tokio::Mutex (lost-update, same class as 2104c030; the daemon only reads that array, so a tray-process lock is sufficient).
    • #7 custom-endpoint reqwest client sets redirect::Policy::none() so a 3xx can't forward the Bearer key.
    • #8 probe::is_fatal now matches the producer's exact phrases ("rejected the API key", "custom provider unreachable", "selected but not configured"), not bare "API key"; added a test for a 400 schema-refusal that mentions a key.
    • #11 the Test button stops Enter/Space from bubbling to the card's onKeyDown.
    • #12 reset_time tries every "reset" occurrence so "resetting" can't shadow a real "reset at 3pm"; added a test.
    • #13 severity is neutralized by [#1] (the resume path is now dev-only); documented the deliberate "resume re-issues one paid call after a crash" tradeoff on runner::exec.
    • Nits: fixed the em-dash in the user-facing timeout string; pre-061 -> pre-064 comments; resolver.rs "a minute later" -> "half an hour"; PR description's migration-062 "additive" claim corrected (it's destructive-but-safe).

    Deferred as follow-ups (happy to file issues / a stacked PR)

    • #9 / [#10] - key/error redaction hardening. A redacting Debug newtype for api_key (#9) touches several structs + their serde, and sanitizing cached backend error text (#10) needs a shared redaction step at the connectivity-test boundary. Both are real hardening but broader than this review pass and read as "consider" - keeping them out to avoid ballooning a feature PR.
    • Low nit - hour.rs/workstream.rs fold + marker two-statement window: left as-is since, as you noted, coalescing makes the re-fold a safe no-op.
    • Coverage gaps you flagged (mock-server test for openai_compat::complete(), behavior-level plan-store/task-composer tests, detect_all_with_cache id-merge): worth doing, tracked as follow-up.

    All local checks green (cargo fmt + clippy + test, UI build + bun test, integration tests). Ready for another look - leaving the merge to you.

     

    Related

    Tickets: #1
    Tickets: #10

  • Anonymous

    Anonymous - 2026-07-18

    Ticket changed by: Akarsh-Hegde

    • status: open --> closed
     

Log in to post a comment.