Skip to content

Enable edit and delete on transcription and embedding models#571

Merged
lucksus merged 2 commits intodevfrom
edit-delete-non-llm-models
Mar 3, 2026
Merged

Enable edit and delete on transcription and embedding models#571
lucksus merged 2 commits intodevfrom
edit-delete-non-llm-models

Conversation

@lucksus
Copy link
Copy Markdown
Member

@lucksus lucksus commented Feb 24, 2025

So we can better play with and test different models especially for finding best whisper model sizes for different GPUs

Summary by CodeRabbit

  • Refactor
    • Restructured action button rendering in model cards for improved layout consistency
    • Removed TRANSCRIPTION model-specific edit button

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 3, 2026

📝 Walkthrough

Walkthrough

The ModelCard component's conditional rendering logic has been restructured. Edit and Delete buttons now consistently render for all model types within an outer action container. LLM-specific controls (Default/Set Default) are nested conditionally, and TRANSCRIPTION-specific button rendering has been removed.

Changes

Cohort / File(s) Summary
ModelCard Component Restructuring
ui/src/components/ModelCard.tsx
Refactored conditional rendering logic: Edit and Delete buttons now always render in outer action flex container; LLM-specific Default/Set Default controls moved to nested conditional; TRANSCRIPTION-specific button block removed entirely. Affects model type-specific UI rendering.
Dependency Adjustments
package.json
Updated dependencies with 6 lines added and 10 lines removed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Buttons dance in flexbox rows,
No more scattered, now it flows,
LLMs get special care,
Edit and Delete everywhere!
Simpler, cleaner UI show~

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title directly aligns with the main change: enabling edit and delete functionality for transcription and embedding models, which matches the refactoring of conditional logic in ModelCard.tsx.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch edit-delete-non-llm-models

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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.

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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ui/src/components/ModelCard.tsx`:
- Around line 74-80: The icon-only action buttons using the j-button elements
that call editModel and removeModel lack accessible names; add descriptive
aria-label attributes to each j-button (e.g., aria-label="Edit model" on the
button with onClick={editModel} and aria-label="Delete model" on the button with
onClick={removeModel}) so screen readers can announce the actions while keeping
the existing icons and handlers intact.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ec3931e and 81ce2a1.

📒 Files selected for processing (1)
  • ui/src/components/ModelCard.tsx

Comment on lines 74 to 80
<j-button variant="subtle" size="sm" onClick={editModel}>
<j-icon name="pencil-square" size="sm" />
</j-button>

<j-button variant="subtle" size="sm" onClick={removeModel}>
<j-icon name="trash" size="sm" />
</j-button>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, locate and examine the ModelCard.tsx file
fd -t f "ModelCard.tsx" --exec cat -n {} \;

Repository: coasys/ad4m

Length of output: 6798


🏁 Script executed:

# Search for j-button component definition
fd -type f \( -name "*.tsx" -o -name "*.ts" -o -name "*.jsx" -o -name "*.js" \) | xargs grep -l "j-button\|export.*button" | head -20

Repository: coasys/ad4m

Length of output: 225


🏁 Script executed:

# Correct fd syntax to search for j-button component definition
fd -t f "\.(tsx?|jsx?)$" | xargs rg "j-button|export.*[Bb]utton" -l | head -20

Repository: coasys/ad4m

Length of output: 679


🏁 Script executed:

# Search for j-button definition or where it's defined
rg "j-button" -t tsx -t ts -A 2 -B 2 | head -50

Repository: coasys/ad4m

Length of output: 82


🏁 Script executed:

# Search for j-button in package.json and index files
rg "j-button" --max-count=10

Repository: coasys/ad4m

Length of output: 7291


🏁 Script executed:

# Check package.json for j-button and related dependencies
cat package.json | grep -A 5 -B 5 "j-button\|juiced"

Repository: coasys/ad4m

Length of output: 37


🏁 Script executed:

# Check if package.json exists and view it
cat package.json

Repository: coasys/ad4m

Length of output: 4949


🏁 Script executed:

# Check ui workspace package.json for j-button dependency
cat ui/package.json | head -100

Repository: coasys/ad4m

Length of output: 2837


🏁 Script executed:

# Search for other icon-only buttons in the codebase to see if they have accessibility attributes
rg "<j-button.*\n.*<j-icon" -A 1 -B 1

Repository: coasys/ad4m

Length of output: 246


🏁 Script executed:

# Search for icon-only buttons (j-button with only j-icon as content)
rg -U "j-button[^>]*>\s*<j-icon" -A 1 -B 1

Repository: coasys/ad4m

Length of output: 2728


🏁 Script executed:

# Check if there are any aria-label or title attributes used with j-button in the codebase
rg "j-button.*aria-label|j-button.*title" -A 2 -B 2

Repository: coasys/ad4m

Length of output: 37


🏁 Script executed:

# Check ActionButton.tsx since it uses j-tooltip with title attribute
cat ui/src/components/ActionButton.tsx

Repository: coasys/ad4m

Length of output: 543


🏁 Script executed:

# Search for j-tooltip usage in the codebase to see the pattern
rg "j-tooltip" -B 2 -A 3 | head -60

Repository: coasys/ad4m

Length of output: 890


🏁 Script executed:

# Check for any aria-* attributes used on j-button throughout the codebase
rg "j-button.*aria-" -A 2 -B 1

Repository: coasys/ad4m

Length of output: 37


🏁 Script executed:

# Check if there are any examples of label text or accessibility patterns with icon buttons
rg "pencil|trash|download|upload" -A 3 -B 1 ui/src/components/ | grep -A 3 -B 1 "j-button"

Repository: coasys/ad4m

Length of output: 2719


Add accessible names to icon-only action buttons.

The Edit (pencil-square) and Delete (trash) buttons at lines 74 and 78 are icon-only and lack accessible names for screen reader users. Add aria-label attributes to identify each action:

Suggested fix
-              <j-button variant="subtle" size="sm" onClick={editModel}>
+              <j-button
+                variant="subtle"
+                size="sm"
+                onClick={editModel}
+                aria-label={`Edit ${name} model`}
+              >
                 <j-icon name="pencil-square" size="sm" />
               </j-button>

-              <j-button variant="subtle" size="sm" onClick={removeModel}>
+              <j-button
+                variant="subtle"
+                size="sm"
+                onClick={removeModel}
+                aria-label={`Delete ${name} model`}
+              >
                 <j-icon name="trash" size="sm" />
               </j-button>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ui/src/components/ModelCard.tsx` around lines 74 - 80, The icon-only action
buttons using the j-button elements that call editModel and removeModel lack
accessible names; add descriptive aria-label attributes to each j-button (e.g.,
aria-label="Edit model" on the button with onClick={editModel} and
aria-label="Delete model" on the button with onClick={removeModel}) so screen
readers can announce the actions while keeping the existing icons and handlers
intact.

@lucksus lucksus merged commit 6dfad94 into dev Mar 3, 2026
5 checks passed
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