Originally created by: adityaharishch
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].
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.src/llm/openai_compat.rs, custom_llm.rs, CustomProviders.tsx).src/llm/probe.rs, schema.rs) and gates on it, so a model that can't honour structured output never silently corrupts the pipeline.src/llm/codex.rs).src/llm/detect.rs), wired into both Settings and the setup wizard, with results cached to disk.src/llm/reset_time.rs), and serialises cache writes so a concurrent "Rescan" can't lose results.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.src/llm_experiment/*, migration 064), a meridian-core reader, dev-only tray commands, and a meridian llm-experiment CLI.WorklogTargets.tsx / WorklogTicketPicker.tsx and a large rework of pm_worklog/generate.rs.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).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/.cargo test passes (new coverage in meridian-core/tests/readers.rs, worklog/llm unit tests)cargo clippy -- -D warnings passescargo fmt --check passescd ui && npm run build) - full pre-push suite green on the latest pushservices/, 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.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.
Originally posted by: coderabbitai[bot]
✨ 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 helpto get the list of available commands.Originally posted by: Akarsh-Hegde
Thanks for the thorough review - pushed
986a307caddressing it. Summary of what changed and what's deliberately deferred.High severity - all fixed
#1 - LLM Lab CLI dev-gating.
run/create/execnow refuse to run in a release binary (they drive real, metered completions throughresolver::backend_for), matchingdraft-taskand the tray'sdev_only().list/getstay open (reads only). Correctedcli.rs's false "no network, no cost" doc.#2 - Worklog multi-ticket idempotency.
upsert_draft/retarget_draftnow run the parent guard-write andtargets::replacein one transaction (targets::replacetakes&mut SqliteConnection), so a concurrentapprove()can't slip a comment-post between the guard matching and the child-table DELETE.begin_create/revert_create+ migration 065'screate_attempt_at), the create-step analog of yourpost_attempt_at. Two concurrent approves - or a crash betweencreate_ticketreturning andmark_createdrecording the key - can no longer each file a real ticket. New tests cover exclusivity, revert-frees-retry, stale-reclaim, and success-resolves.post_attempt_at, documented in migration 065 andbegin_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 surfacesoutcome_unknownin the UI; anapprovedrow 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 likeoutcome_unknownif 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.plan_tasks/create.rseach passingensure_plan_has_roomand 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 nowskipsbase_urltoo (a URL can carry a key in a query string).endpoint_id+vendorremain 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_backoffpublic API (now test-only). New test asserts a Claude backoff doesn't divert a switched-to Codex.Medium / nits - fixed
add/probe/removeserialize theirsettings.jsonread-modify-write behind atokio::Mutex(lost-update, same class as 2104c030; the daemon only reads that array, so a tray-process lock is sufficient).redirect::Policy::none()so a 3xx can't forward the Bearer key.probe::is_fatalnow 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.Enter/Spacefrom bubbling to the card'sonKeyDown.reset_timetries every"reset"occurrence so"resetting"can't shadow a real"reset at 3pm"; added a test.runner::exec.pre-061->pre-064comments;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)
Debugnewtype forapi_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.hour.rs/workstream.rsfold + marker two-statement window: left as-is since, as you noted, coalescing makes the re-fold a safe no-op.openai_compat::complete(), behavior-level plan-store/task-composer tests,detect_all_with_cacheid-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:
#1Tickets:
#10Ticket changed by: Akarsh-Hegde