Skip to content

Run config details dialog#804

Merged
sfierro merged 1 commit into
sfierro/rag_evalsfrom
sfierro/run_config_dialog
Nov 13, 2025
Merged

Run config details dialog#804
sfierro merged 1 commit into
sfierro/rag_evalsfrom
sfierro/run_config_dialog

Conversation

@sfierro
Copy link
Copy Markdown
Contributor

@sfierro sfierro commented Nov 13, 2025

dialog.mov

Summary by CodeRabbit

  • New Features
    • Added a detailed modal dialog for viewing run configuration details, displaying model information, prompts, available tools, and configuration parameters.
    • Introduced an improved run configuration summary card with clickable access to detailed information and visual indicators for default configurations.

@sfierro sfierro requested review from leonardmq and scosman November 13, 2025 06:41
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 13, 2025

Walkthrough

This PR introduces two new Svelte components for run configuration display: run_config_details_dialog (modal dialog) and run_config_summary (summary card), along with a new utility function getRunConfigUiProperties to assemble UI properties. Existing inline run config display logic is refactored to use these new components in the comparison and settings pages.

Changes

Cohort / File(s) Summary
New Run Config UI Components
app/web_ui/src/lib/ui/run_config_component/run_config_details_dialog.svelte, app/web_ui/src/lib/ui/run_config_component/run_config_summary.svelte
Introduces two new components: run_config_details_dialog exports project_id, task_id, task_run_config props and show() function; run_config_summary exports props for project_id, task_id, task_run_config, is_default and renders a clickable card that opens the details dialog.
Run Config Utilities
app/web_ui/src/lib/utils/run_config_formatters.ts
Adds getRunConfigUiProperties function that assembles UI properties (model, prompt, tools, basic config fields) from run config data, model info, prompts, and available tools.
Run Configs Comparison Page
app/web_ui/src/routes/(app)/evals/[project_id]/[task_id]/[eval_id]/compare_run_configs/+page.svelte
Replaces inline prompt/model display logic with new RunConfigSummary component; removes direct imports of getDetailedModelName, getRunConfigPromptDisplayName, getRunConfigPromptInfoText, and prompt_link.
Tools Management Settings Page
app/web_ui/src/routes/(app)/settings/manage_tools/[project_id]/kiln_task/[tool_server_id]/+page.svelte
Removes imports of ProviderModels, PromptResponse, model_name, provider_name_from_id, get_tools_property_info, and getRunConfigPromptDisplayName; adds getRunConfigUiProperties and introduces task_id_for_run_config state to drive property computation.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant RunConfigSummary
    participant RunConfigDetailsDialog
    participant getRunConfigUiProperties
    participant Store/Utils

    User->>RunConfigSummary: Click summary card
    RunConfigSummary->>RunConfigDetailsDialog: show()
    RunConfigDetailsDialog->>getRunConfigUiProperties: Compute properties
    getRunConfigUiProperties->>Store/Utils: Fetch model_info, prompts, tools
    Store/Utils-->>getRunConfigUiProperties: Return data
    getRunConfigUiProperties-->>RunConfigDetailsDialog: Return UiProperty[]
    RunConfigDetailsDialog-->>User: Display modal with properties
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Component composition logic: Verify proper prop passing and bindings in new components, especially bind:this pattern in run_config_summary for dialog access
  • Property assembly function: Review getRunConfigUiProperties for correct handling of null/undefined values, tool property fetching, and property object structure
  • Refactoring correctness: Ensure migration from inline logic to RunConfigSummary component in both page files preserves original rendering behavior
  • Store integration: Validate reactive computations using prompts_store and model_info in the new components

Possibly related PRs

Suggested labels

codex

Suggested reviewers

  • scosman
  • aiyakitori
  • leonardmq

Poem

🐰 A summary card with details to show,
Click and a modal steals the flow,
Properties assembled, neat and clean,
The finest run configs you've seen! ✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is largely incomplete, containing only a video link without following the required template structure or providing text-based explanation of changes. Fill out the PR description template with: 'What does this PR do?' section, 'Related Issues' link, CLA confirmation, and completion of both checklist items.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: introducing a new dialog component for displaying run configuration details.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sfierro/run_config_dialog

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.

@github-actions
Copy link
Copy Markdown

📊 Coverage Report

Overall Coverage: 92%

Diff: origin/sfierro/rag_evals...HEAD

No lines with coverage information in this diff.


Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
app/web_ui/src/lib/utils/run_config_formatters.ts (1)

144-144: Consider omitting badge: false for cleaner code.

The explicit badge: false assignment is unnecessary since false is typically the default/absence state. You can simplify by only including the badge property when it's true:

     {
       name: "Available Tools",
       value: tools_property_info.value,
       links: tools_property_info.links,
-      badge: Array.isArray(tools_property_info.value) ? true : false,
+      ...(Array.isArray(tools_property_info.value) && { badge: true }),
     },
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9f7e550 and cedf0e3.

📒 Files selected for processing (5)
  • app/web_ui/src/lib/ui/run_config_component/run_config_details_dialog.svelte (1 hunks)
  • app/web_ui/src/lib/ui/run_config_component/run_config_summary.svelte (1 hunks)
  • app/web_ui/src/lib/utils/run_config_formatters.ts (2 hunks)
  • app/web_ui/src/routes/(app)/evals/[project_id]/[task_id]/[eval_id]/compare_run_configs/+page.svelte (2 hunks)
  • app/web_ui/src/routes/(app)/settings/manage_tools/[project_id]/kiln_task/[tool_server_id]/+page.svelte (4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
app/web_ui/**/*.{ts,svelte}

📄 CodeRabbit inference engine (.cursor/rules/project.mdc)

app/web_ui/**/*.{ts,svelte}: Write frontend web code in TypeScript; do not add plain .js sources
From the web UI, make backend calls only to our FastAPI servers; do not call external services directly
Use TypeScript types and interfaces to maintain strong typing in the frontend

Files:

  • app/web_ui/src/lib/utils/run_config_formatters.ts
  • app/web_ui/src/routes/(app)/evals/[project_id]/[task_id]/[eval_id]/compare_run_configs/+page.svelte
  • app/web_ui/src/lib/ui/run_config_component/run_config_summary.svelte
  • app/web_ui/src/routes/(app)/settings/manage_tools/[project_id]/kiln_task/[tool_server_id]/+page.svelte
  • app/web_ui/src/lib/ui/run_config_component/run_config_details_dialog.svelte
app/web_ui/**/*.{ts,tsx,svelte}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript for frontend code (including <script lang="ts"> in .svelte files)

Files:

  • app/web_ui/src/lib/utils/run_config_formatters.ts
  • app/web_ui/src/routes/(app)/evals/[project_id]/[task_id]/[eval_id]/compare_run_configs/+page.svelte
  • app/web_ui/src/lib/ui/run_config_component/run_config_summary.svelte
  • app/web_ui/src/routes/(app)/settings/manage_tools/[project_id]/kiln_task/[tool_server_id]/+page.svelte
  • app/web_ui/src/lib/ui/run_config_component/run_config_details_dialog.svelte
app/web_ui/**/*.svelte

📄 CodeRabbit inference engine (.cursor/rules/project.mdc)

app/web_ui/**/*.svelte: Author components for Svelte v4 compatibility (do not use Svelte v5-only APIs)
Prefer Tailwind CSS and DaisyUI utility/classes for styling in Svelte components

Files:

  • app/web_ui/src/routes/(app)/evals/[project_id]/[task_id]/[eval_id]/compare_run_configs/+page.svelte
  • app/web_ui/src/lib/ui/run_config_component/run_config_summary.svelte
  • app/web_ui/src/routes/(app)/settings/manage_tools/[project_id]/kiln_task/[tool_server_id]/+page.svelte
  • app/web_ui/src/lib/ui/run_config_component/run_config_details_dialog.svelte
🧠 Learnings (4)
📚 Learning: 2025-10-07T00:21:33.537Z
Learnt from: sfierro
Repo: Kiln-AI/Kiln PR: 698
File: app/web_ui/src/routes/(app)/evals/[project_id]/[task_id]/compare/+page.svelte:800-812
Timestamp: 2025-10-07T00:21:33.537Z
Learning: In the Kiln web UI codebase, the `save_new_task_run_config` function in `app/web_ui/src/lib/stores/run_configs_store.ts` automatically refreshes the run configs store by calling `load_task_run_configs(project_id, task_id, true)` with force_refresh=true after successfully creating a new run config. This means callers of save_new_task_run_config do not need to manually refresh the store afterward, as the store will already contain the newly created config.

Applied to files:

  • app/web_ui/src/lib/utils/run_config_formatters.ts
  • app/web_ui/src/routes/(app)/evals/[project_id]/[task_id]/[eval_id]/compare_run_configs/+page.svelte
  • app/web_ui/src/lib/ui/run_config_component/run_config_summary.svelte
  • app/web_ui/src/routes/(app)/settings/manage_tools/[project_id]/kiln_task/[tool_server_id]/+page.svelte
📚 Learning: 2025-11-11T00:07:51.525Z
Learnt from: sfierro
Repo: Kiln-AI/Kiln PR: 756
File: app/web_ui/src/lib/ui/run_config_component/tools_selector.svelte:19-20
Timestamp: 2025-11-11T00:07:51.525Z
Learning: In app/web_ui/src/lib/ui/run_config_component/tools_selector.svelte, the single_select_selected_tool prop should NOT be synchronized with the tools_store. Only the multi-select tools array should persist to the store. The single_select mode is intended to use ephemeral component-local state.

Applied to files:

  • app/web_ui/src/lib/ui/run_config_component/run_config_summary.svelte
  • app/web_ui/src/routes/(app)/settings/manage_tools/[project_id]/kiln_task/[tool_server_id]/+page.svelte
📚 Learning: 2025-09-25T07:20:11.459Z
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 650
File: app/web_ui/src/routes/(app)/docs/library/[project_id]/[document_id]/+page.svelte:19-19
Timestamp: 2025-09-25T07:20:11.459Z
Learning: When analyzing relative import paths in SvelteKit route structures, be more careful to verify the actual directory structure exists before suggesting corrections. The import path ../../../../generate/[project_id]/[task_id]/table_button.svelte from app/web_ui/src/routes/(app)/docs/library/[project_id]/[document_id]/+page.svelte correctly resolves to the existing file at app/web_ui/src/routes/(app)/generate/[project_id]/[task_id]/table_button.svelte.

Applied to files:

  • app/web_ui/src/routes/(app)/settings/manage_tools/[project_id]/kiln_task/[tool_server_id]/+page.svelte
📚 Learning: 2025-10-29T11:19:31.059Z
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 759
File: app/web_ui/src/lib/api_schema.d.ts:2485-2490
Timestamp: 2025-10-29T11:19:31.059Z
Learning: In libs/server/kiln_server/document_api.py and the generated app/web_ui/src/lib/api_schema.d.ts, CreateVectorStoreConfigRequest.properties is now a union of three distinct public schemas: LanceDBConfigFTSPropertiesPublic, LanceDBConfigVectorPropertiesPublic, and LanceDBConfigHybridPropertiesPublic. These are intentionally split to allow independent evolution; do not suggest merging them.

Applied to files:

  • app/web_ui/src/routes/(app)/settings/manage_tools/[project_id]/kiln_task/[tool_server_id]/+page.svelte
🧬 Code graph analysis (1)
app/web_ui/src/lib/utils/run_config_formatters.ts (5)
app/web_ui/src/lib/types.ts (4)
  • TaskRunConfig (34-34)
  • ProviderModels (14-14)
  • PromptResponse (26-26)
  • ToolSetApiDescription (82-83)
app/web_ui/src/lib/stores.ts (4)
  • model_info (317-317)
  • available_tools (189-191)
  • model_name (410-423)
  • provider_name_from_id (478-495)
app/web_ui/src/lib/ui/property_list.ts (1)
  • UiProperty (1-10)
app/web_ui/src/lib/utils/link_builder.ts (1)
  • prompt_link (3-25)
app/web_ui/src/lib/stores/tools_store.ts (1)
  • get_tools_property_info (16-34)
⏰ 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). (6)
  • GitHub Check: Generate Coverage Report
  • GitHub Check: Build Desktop Apps (macos-15-intel)
  • GitHub Check: Build Desktop Apps (ubuntu-22.04-arm)
  • GitHub Check: Build Desktop Apps (macos-latest)
  • GitHub Check: Build Desktop Apps (ubuntu-22.04)
  • GitHub Check: Build Desktop Apps (windows-latest)
🔇 Additional comments (11)
app/web_ui/src/lib/utils/run_config_formatters.ts (2)

1-14: LGTM!

The imports are well-organized and appropriate for the new functionality. All necessary types and utilities are correctly imported from internal modules.


93-119: LGTM!

The function signature and initial computations are well-structured. The consistent use of "Loading..." fallbacks provides a good user experience while data is being fetched, and null handling is properly implemented throughout.

app/web_ui/src/routes/(app)/settings/manage_tools/[project_id]/kiln_task/[tool_server_id]/+page.svelte (2)

29-29: LGTM!

The import has been correctly updated to use the new getRunConfigUiProperties function, which consolidates the run config UI data assembly.


50-50: LGTM!

The refactoring correctly introduces task_id_for_run_config to coordinate the loading of run config properties. The reactive statement properly waits for both task_id_for_run_config and run_config to be available before computing properties, and provides an appropriate fallback of an empty array when they're not ready.

Also applies to: 95-95, 220-237

app/web_ui/src/lib/ui/run_config_component/run_config_summary.svelte (3)

1-22: LGTM!

The script section is well-structured with proper TypeScript usage, correctly typed exports, and appropriate reactive declarations. The component binding pattern for the dialog is compatible with Svelte v4.


24-50: LGTM!

The template properly uses Tailwind CSS and DaisyUI classes for styling as per the coding guidelines. The click handler and store subscriptions are correctly implemented for Svelte v4 compatibility.


52-57: LGTM!

The dialog component binding is correctly implemented using bind:this, which is the proper Svelte v4 pattern for accessing child component methods. Props are appropriately passed to the dialog.

app/web_ui/src/routes/(app)/evals/[project_id]/[task_id]/[eval_id]/compare_run_configs/+page.svelte (2)

42-42: LGTM!

The import correctly adds the new RunConfigSummary component for use in the table.


587-593: LGTM!

The refactoring correctly replaces inline rendering with the new RunConfigSummary component. All required props are properly passed, including the correctly computed is_default flag.

app/web_ui/src/lib/ui/run_config_component/run_config_details_dialog.svelte (2)

1-33: LGTM!

The script section follows best practices with proper TypeScript usage, correctly typed exports, and appropriate reactive declarations. The exported show() function allows parent components to programmatically open the dialog, and all patterns are Svelte v4 compatible.


35-37: LGTM!

The template is clean and correctly structured. The Dialog component binding and prop passing are proper Svelte v4 patterns, and the PropertyList component receives the computed properties as expected.

@sfierro sfierro merged commit a04ada5 into sfierro/rag_evals Nov 13, 2025
15 checks passed
@sfierro sfierro deleted the sfierro/run_config_dialog branch November 13, 2025 07:44
@coderabbitai coderabbitai Bot mentioned this pull request Jan 23, 2026
@coderabbitai coderabbitai Bot mentioned this pull request Feb 20, 2026
This was referenced Mar 9, 2026
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.

2 participants