fix(tui): restore backspace in AgentURL (eval field 0)

[puwun]

Description

Restores backspace functionality in the AgentURL field on the New Evaluation form

Type of Change

• 🐛 Bug fix (non-breaking change which fixes an issue)

Changes Made

Modified the guard condition in the backspace handler within handleEvalFormInput function in eval_form_controller.go

Related Issues/PRs

• Fixes [Bug] - Input handler logic error prevents character input in AgentURL #125

[coderabbitai]

Summary by CodeRabbit

• Bug Fixes
  • Fixed backspace functionality to properly handle text deletion in all input fields, including the first field and fields with enhanced validation guards.

Walkthrough

Backspace handling in the TUI evaluation form was adjusted so deletions are allowed when currentField is 0. Guards were added for AgentURL (field 0) and JudgeModel (field 3) to ensure cursorPos and rune bounds are validated before deleting.

Changes

Cohort / File(s)	Change Summary
Backspace handler adjustments packages/tui/internal/tui/eval_form_controller.go	Modified backspace condition to permit deletion when currentField == 0; added guards for text cases (field 0 AgentURL and field 3 JudgeModel) validating cursorPos > 0, cursorPos <= len(runes), and non-empty field before performing backspace deletion.

Sequence Diagram(s)

sequenceDiagram
  participant TUI
  participant EvalFormCtrl as EvalFormController
  note right of EvalFormCtrl #f0f8ff: Backspace handling branch updates
  TUI->>EvalFormCtrl: Backspace key
  EvalFormCtrl->>EvalFormCtrl: check currentField >= 0
  alt currentField == 0 (AgentURL)
    EvalFormCtrl->>EvalFormCtrl: ensure cursorPos > 0 && cursorPos <= len(runes) && field non-empty
    alt guard passes
      EvalFormCtrl->>EvalFormCtrl: remove rune at cursorPos-1
      EvalFormCtrl->>TUI: update field rendering
    else guard fails
      EvalFormCtrl->>TUI: no-op (ignore backspace)
    end
  else currentField == 3 (JudgeModel)
    EvalFormCtrl->>EvalFormCtrl: same cursor/rune guards and deletion
    EvalFormCtrl->>TUI: update field rendering / no-op
  else other fields
    EvalFormCtrl->>EvalFormCtrl: existing backspace logic (unchanged)
    EvalFormCtrl->>TUI: update field rendering / no-op
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Single-file change with a small logic tweak and added guards.
• Areas to review closely:
  • Correctness of cursor boundary checks (off-by-one concerns).
  • Behavior for empty strings and cursor at position 0.
  • Interaction with any paste/insert logic or MCP-related code paths.

Possibly related PRs

• hotfix/slash agent url #106: Modifies input handling for the same evaluation form fields (AgentURL and JudgeModel), likely interacting with insertion/cursor logic changed here.

Suggested reviewers

• yuval-qf

Poem

🐰
Tap-tap the keys, a bug undone,
Backspace hops back, the fix is fun.
First field freed, the cursor sings,
URL trimmed with nimble springs.
Hooray — the form now happily runs! 🥕

Pre-merge checks and finishing touches

❌ 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%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The PR title "fix(tui): restore backspace in AgentURL (eval field 0)" is specific and clear, directly summarizing the main change. It identifies the type of change (fix), the component affected (tui), and the specific issue being resolved (backspace in AgentURL field 0). The title is concise and provides sufficient context for someone scanning the repository history to understand the primary change without needing to review the full description.
Linked Issues Check	✅ Passed	The PR directly addresses the objective stated in issue #125. The issue identified that backspace functionality was broken for the AgentURL field (field 0) due to a guard condition that was too restrictive. The code changes documented in the raw summary show the condition was modified from checking currentField > 0 to currentField >= 0, which directly enables backspace handling for field 0. Additionally, guards were added to ensure proper validation before deletion. This implementation aligns with the stated objective to restore backspace functionality for the AgentURL field.
Out of Scope Changes Check	✅ Passed	All changes documented in the PR are directly scoped to fixing the backspace issue in the AgentURL field (field 0). The modifications include adjusting the guard condition in the backspace handler and adding appropriate validation checks for specific field cases. There are no changes to unrelated functionality, dependencies, documentation, or other components outside the scope of fixing the reported bug in eval_form_controller.go.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 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 058f90d and 4226010.

📒 Files selected for processing (1)

• packages/tui/internal/tui/eval_form_controller.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/tui/internal/tui/eval_form_controller.go

⏰ 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). (1)

• GitHub Check: codestyle

---

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
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/tui/internal/tui/eval_form_controller.go (1)

102-113: Backspace at cursor position 0 will cause a panic.

The deletion logic for fields 0 (AgentURL) and 3 (JudgeModel) does not check if cursorPos > 0 before performing runes[:cursorPos-1] on lines 105 and 111. If a user presses backspace when the cursor is at the beginning of the field (position 0), this will cause a slice index out of range panic.

Apply this diff to fix the issue:

 		case 0: // AgentURL
 			runes := []rune(m.evalState.AgentURL)
-			if m.evalState.cursorPos <= len(runes) {
+			if m.evalState.cursorPos > 0 && m.evalState.cursorPos <= len(runes) {
 				m.evalState.AgentURL = string(runes[:m.evalState.cursorPos-1]) + string(runes[m.evalState.cursorPos:])
 				m.evalState.cursorPos--
 			}
 		case 3: // JudgeModel
 			runes := []rune(m.evalState.JudgeModel)
-			if m.evalState.cursorPos <= len(runes) {
+			if m.evalState.cursorPos > 0 && m.evalState.cursorPos <= len(runes) {
 				m.evalState.JudgeModel = string(runes[:m.evalState.cursorPos-1]) + string(runes[m.evalState.cursorPos:])
 				m.evalState.cursorPos--
 			}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fc46a7e and 058f90d.

📒 Files selected for processing (1)

• packages/tui/internal/tui/eval_form_controller.go (1 hunks)

⏰ 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). (1)

• GitHub Check: codestyle

🔇 Additional comments (1)

packages/tui/internal/tui/eval_form_controller.go (1)

114-121: LGTM! ParallelRuns backspace logic is safe.

The deletion logic for field 6 (ParallelRuns) properly guards against invalid states with checks for values >= 10 and > 0 before performing operations. This prevents any potential panics or invalid states.

[yuval-qf]

LGTM!
thanks for the fix @puwun