Skip to content

chore: add provider connected snapshot event#837

Open
leonardmq wants to merge 1 commit intomainfrom
leonard/kil-203-add-analytics-event-for-connected_providers
Open

chore: add provider connected snapshot event#837
leonardmq wants to merge 1 commit intomainfrom
leonard/kil-203-add-analytics-event-for-connected_providers

Conversation

@leonardmq
Copy link
Collaborator

@leonardmq leonardmq commented Nov 20, 2025

What does this PR do?

Add new analytics event in { provider_id: bool } format.

The event is sent when the user visits the connect_providers page; as well as within the page whenever the list of active provider changes.

One problem is visiting without Ollama or DMR turned on is treated as meaning they are not connected, but not sure if we can avoid this.

Event shape:

{
    "ollama": true,
    "docker_model_runner": true,
    "openai": false,
    "openrouter": true,
    "groq": true,
    "amazon_bedrock": true,
    "fireworks_ai": true,
    "anthropic": false,
    "vertex": true,
    "gemini_api": false,
    "huggingface": true,
    "azure_openai": false,
    "together_ai": true,
    "openai_compatible": false,
    "wandb": true,
    "siliconflow_cn": true,
    "cerebras": true
}

Checklists

  • Tests have been run locally and passed
  • New tests have been added to any work in /lib

Summary by CodeRabbit

  • Chores
    • Improved reliability of provider connection initialization with enhanced async handling.
    • Added telemetry to monitor provider connection states for better platform insights.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 20, 2025

Walkthrough

A provider setup component is enhanced with per-user provider connection telemetry and refactored async handling. Promise chains using .then() are converted to await syntax, and a reactive telemetry block is added that only fires after initial provider loading completes, preventing telemetry during initialization.

Changes

Cohort / File(s) Summary
Provider telemetry and async refactoring
app/web_ui/src/routes/(fullscreen)/setup/(setup)/connect_providers/connect_providers.svelte
Added initial_providers_loaded flag to gate telemetry; converted onMount promise chains to async/await; introduced reactive block to post per-user provider connection state to PostHog under "connect_providers_per_user" event after initial load; maintained existing error handling and UI update behavior

Sequence Diagram

sequenceDiagram
    participant Component as Component Init
    participant Providers as Provider Connections
    participant Flag as initial_providers_loaded
    participant Telemetry as Telemetry Block

    Component->>Providers: onMount: connect ollama & docker
    Providers->>Providers: await both connections
    Providers->>Flag: Set initial_providers_loaded = true
    Note over Telemetry: Reactive block gated by flag
    Flag->>Telemetry: Flag change detected
    Telemetry->>Telemetry: Capture provider state snapshot
    Telemetry->>Telemetry: Post to PostHog event
    
    Note over Component,Telemetry: Subsequent provider changes trigger telemetry<br/>(only after initial load completes)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–25 minutes

  • Verify that converting .then() chains to await maintains existing behavior and error handling semantics
  • Confirm the initial_providers_loaded guard correctly prevents telemetry from firing during component initialization
  • Review telemetry event structure and ensure the captured provider state snapshot is correct
  • Validate that reactive block triggers appropriately on provider state changes after the flag is set

Poem

🐰 Hop! The providers connect with grace,
Telemetry now tracking pace,
Await, not then—a cleaner way,
No early metrics lead astray,
One flag to guard the data flow! 🔔

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding a provider connected snapshot event for analytics purposes.
Description check ✅ Passed The description covers the main purpose, provides specific event details with an example payload, acknowledges a known limitation, and confirms both local testing and new tests were added.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch leonard/kil-203-add-analytics-event-for-connected_providers

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 819d930 and 05e77c9.

📒 Files selected for processing (1)
  • app/web_ui/src/routes/(fullscreen)/setup/(setup)/connect_providers/connect_providers.svelte (3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 546
File: app/web_ui/src/routes/(app)/docs/library/[project_id]/upload_file_dialog.svelte:107-120
Timestamp: 2025-09-10T08:32:18.688Z
Learning: leonardmq prefers to work within the constraints of their SDK codegen for API calls, even when typing is awkward (like casting FormData to match expected types), rather than using alternative approaches like native fetch that would cause compiler errors with their generated types.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 413
File: libs/core/kiln_ai/datamodel/chunk.py:138-160
Timestamp: 2025-08-22T11:17:56.862Z
Learning: leonardmq prefers to avoid redundant validation checks when upstream systems already guarantee preconditions are met. He trusts the attachment system to ensure paths are properly formatted and prefers letting the attachment method (resolve_path) handle any edge cases rather than adding defensive precondition checks.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 390
File: libs/core/kiln_ai/adapters/provider_tools.py:494-499
Timestamp: 2025-07-23T08:58:45.769Z
Learning: leonardmq prefers to keep tightly coupled implementation details (like API versions) hardcoded when they are not user-facing and could break other modules if changed. The shared Config class is reserved for user-facing customizable values, not internal implementation details.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 341
File: libs/server/kiln_server/document_api.py:44-51
Timestamp: 2025-06-18T08:22:58.510Z
Learning: leonardmq prefers to defer fixing blocking I/O in async handlers when: the operation is very fast (milliseconds), user-triggered rather than automated, has no concurrency concerns, and would require additional testing to fix properly. He acknowledges such issues as valid but makes pragmatic decisions about timing the fixes.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 487
File: libs/core/kiln_ai/datamodel/rag.py:33-35
Timestamp: 2025-09-04T06:45:44.212Z
Learning: leonardmq requires vector_store_config_id to be a mandatory field in RagConfig (similar to extractor_config_id, chunker_config_id, embedding_config_id) for consistency. He prefers fixing dependent code that breaks due to missing required fields rather than making fields optional to accommodate incomplete data.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 654
File: app/web_ui/src/routes/(app)/docs/rag_configs/[project_id]/create_rag_config/edit_rag_config_form.svelte:141-152
Timestamp: 2025-09-25T06:38:14.854Z
Learning: leonardmq prefers simple onMount initialization patterns over reactive statements when possible, and is cautious about maintaining internal state for idempotency in Svelte components. He values simplicity and safety in component lifecycle management.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 402
File: libs/core/kiln_ai/adapters/embedding/litellm_embedding_adapter.py:0-0
Timestamp: 2025-07-14T03:43:07.283Z
Learning: leonardmq prefers to keep defensive validation checks even when they're technically redundant, viewing them as useful "quick sanity checks" that provide additional safety nets. He values defensive programming over strict DRY (Don't Repeat Yourself) principles when the redundant code serves as a safeguard.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 546
File: app/web_ui/src/lib/api_schema.d.ts:0-0
Timestamp: 2025-09-10T08:27:47.227Z
Learning: leonardmq correctly identifies auto-generated files and prefers not to manually edit them. When suggesting changes to generated TypeScript API schema files, the fix should be made in the source OpenAPI specification in the backend, not in the generated TypeScript file.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 487
File: libs/core/kiln_ai/adapters/vector_store/lancedb_adapter.py:136-150
Timestamp: 2025-09-04T06:34:12.318Z
Learning: leonardmq prefers brute force re-insertion of all chunks when partial chunks exist in the LanceDB vector store, rather than selectively inserting only missing chunks. His reasoning is that documents typically have only dozens of chunks (making overwriting cheap), and partial insertion likely indicates data corruption or conflicts that should be resolved by re-inserting all chunks to ensure consistency.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 487
File: libs/core/kiln_ai/adapters/vector_store/lancedb_adapter.py:98-103
Timestamp: 2025-09-04T07:08:04.248Z
Learning: leonardmq prefers to let TableNotFoundError bubble up in delete_nodes_by_document_id operations rather than catching it, as this error indicates operations are being done in the wrong order on the caller side and should be surfaced as a programming error rather than handled gracefully.
📚 Learning: 2025-10-22T04:31:20.500Z
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 739
File: app/web_ui/src/routes/(app)/generate/[project_id]/[task_id]/synth/+page.svelte:170-177
Timestamp: 2025-10-22T04:31:20.500Z
Learning: In Svelte components for the Kiln project, leonardmq prefers tracking previous query parameter values and comparing them in reactive blocks (rather than just checking for existence) to avoid duplicate execution on initial load while still reacting to subsequent navigation changes. This pattern supports both current behavior and future feature additions like mode switching.

Applied to files:

  • app/web_ui/src/routes/(fullscreen)/setup/(setup)/connect_providers/connect_providers.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Generate Coverage Report
  • GitHub Check: Build Desktop Apps (windows-latest)
  • GitHub Check: Build Desktop Apps (ubuntu-22.04)
  • GitHub Check: Build Desktop Apps (macos-15-intel)
🔇 Additional comments (3)
app/web_ui/src/routes/(fullscreen)/setup/(setup)/connect_providers/connect_providers.svelte (3)

675-675: LGTM: Clear flag for gating telemetry after initial load.

The initial_providers_loaded flag correctly prevents telemetry from firing during component initialization.


752-761: LGTM: Await conversion ensures proper sequencing.

Converting from fire-and-forget .then() chains to await correctly ensures initial_providers_loaded is only set to true after both initial provider checks complete. This prevents race conditions where the telemetry block might fire before initial loading finishes.

The .then() callbacks that clear error states are preserved and will still execute after the awaited promises resolve.


364-372: Critical: Svelte reactivity issue prevents telemetry from firing on status changes.

This reactive block likely won't fire after initial load when provider connections change. In Svelte, reactive statements depend on assignments to tracked variables, not nested property mutations. Throughout this file, status is mutated via direct property assignments like status.ollama.connected = true (lines 407, 500, 656, etc.) without reassigning the root status object.

Since Svelte doesn't detect these mutations, this reactive block will only fire once when initial_providers_loaded becomes true, defeating the stated purpose of capturing changes "whenever the list of active providers changes while on that page."

Apply this pattern after each status mutation to trigger reactivity:

 status[provider.id].connected = false
+status = status

Or consolidate mutations into a helper function that ensures reassignment:

function updateProviderStatus(providerId: string, updates: Partial<ProviderStatus>) {
  status[providerId] = { ...status[providerId], ...updates }
  status = status
}

Then use: updateProviderStatus('ollama', { connected: true })

Note: The pre-existing reactive statement at lines 354-356 has the same issue, suggesting has_connected_providers may also not update reliably.

⛔ Skipped due to learnings
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 739
File: app/web_ui/src/routes/(app)/generate/[project_id]/[task_id]/synth/+page.svelte:170-177
Timestamp: 2025-10-22T04:31:20.500Z
Learning: In Svelte components for the Kiln project, leonardmq prefers tracking previous query parameter values and comparing them in reactive blocks (rather than just checking for existence) to avoid duplicate execution on initial load while still reacting to subsequent navigation changes. This pattern supports both current behavior and future feature additions like mode switching.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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

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

@leonardmq leonardmq requested a review from scosman November 20, 2025 09:32
@github-actions
Copy link

📊 Coverage Report

Overall Coverage: 92%

Diff: origin/main...HEAD

No lines with coverage information in this diff.


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant