Skip to content

fix(settings): validate condenser Max Size and block invalid values#925

Open
VascoSch92 wants to merge 1 commit into
mainfrom
863-bug-condenser-max-number-of-events-indicator-can-go-under-0
Open

fix(settings): validate condenser Max Size and block invalid values#925
VascoSch92 wants to merge 1 commit into
mainfrom
863-bug-condenser-max-number-of-events-indicator-can-go-under-0

Conversation

@VascoSch92
Copy link
Copy Markdown
Member

@VascoSch92 VascoSch92 commented May 29, 2026

  • A human has tested these changes.

Summary

Fixes #863. The Condenser "Max Size" field accepted values the backend rejects (negatives, below the minimum, non-integers), so users could only discover the problem via an unclear error when saving.

The backend SDK (CondenserSettings.max_size) enforces ge=20 with no upper bound, so any value below 20 — including the spinner going under 0 — fails validation on save. This change surfaces those rules in the UI as immediate, localized feedback.

Issue Number

#863

Changes

  • Constrain the input. Added a condenser.max_size entry to FIELD_METADATA (min: 20, step: 1), mirroring the existing llm.top_p / llm.temperature constraints. This floors the
    native number spinner at 20 and matches the backend ge=20.
    • Live validation feedback. SchemaField now shows an inline red message as the user types, via the existing SettingsInput error prop (the same pattern used in backend-form-modal /
      edit-automation-modal):
      • below the minimum → "Value must be at least 20."
      • non-integer (e.g. 0.9) → "Please enter a whole number."
      • non-numeric entry (e.g. a typed letter) → "Please enter a whole number." Detected via the input's native validity.badInput flag, read from a raw input event listener (React's
      • non-numeric entry (e.g. a typed letter) → "Please enter a whole number." Detected via the input's native validity.badInput flag, read from a raw input event listener (React's onChange is skipped when a number input's value
        stays empty, so it can't be relied on here).
    • i18n. Added SCHEMA$ERROR$WHOLE_NUMBER and SCHEMA$ERROR$MIN_VALUE (with {{min}} interpolation) across all 15 locales.

Video/Screenshots

Screen.Recording.2026-05-29.at.12.33.40.mov

Type

  • Bug fix
  • Feature
  • Refactor
  • Breaking change
  • Docs / chore

Notes

  • The validation is generic to numeric SchemaFields, not condenser-specific.
  • The min: 20 floor is hardcoded on the frontend because the schema endpoint doesn't expose pydantic's min/max; a comment links it to the SDK constraint so it stays in sync.
  • Browser caveat: in Chrome, type="number" blocks letter keystrokes outright, so the non-numeric message only appears in browsers that allow typing them (Firefox/Safari); floats and below-minimum work everywhere.

🐳 Docker images for this PR

GHCR package: https://github.com/OpenHands/agent-canvas/pkgs/container/agent-canvas

Component Value
Image ghcr.io/openhands/agent-canvas
Architectures amd64, arm64
Agent Server ghcr.io/openhands/agent-server:1.24.0-python
Automation openhands-automation==1.0.0a5
Commit a5d083896fd2aaea4cdb166b6b4005ae45be2380

Pull (multi-arch manifest)

# Multi-arch manifest — Docker automatically pulls the correct architecture
docker pull ghcr.io/openhands/agent-canvas:sha-a5d0838

Run

docker run -it --rm \
  -p 8000:8000 \
  ghcr.io/openhands/agent-canvas:sha-a5d0838

All tags pushed for this build

ghcr.io/openhands/agent-canvas:sha-a5d0838-amd64
ghcr.io/openhands/agent-canvas:863-bug-condenser-max-number-of-events-indicator-can-go-under-0-amd64
ghcr.io/openhands/agent-canvas:pr-925-amd64
ghcr.io/openhands/agent-canvas:sha-a5d0838-arm64
ghcr.io/openhands/agent-canvas:863-bug-condenser-max-number-of-events-indicator-can-go-under-0-arm64
ghcr.io/openhands/agent-canvas:pr-925-arm64
ghcr.io/openhands/agent-canvas:sha-a5d0838
ghcr.io/openhands/agent-canvas:863-bug-condenser-max-number-of-events-indicator-can-go-under-0
ghcr.io/openhands/agent-canvas:pr-925

About Multi-Architecture Support

  • Each tag (e.g., sha-a5d0838) is a multi-arch manifest supporting both amd64 and arm64
  • Docker automatically pulls the correct architecture for your platform
  • Individual architecture tags (e.g., sha-a5d0838-amd64) are also available if needed

@VascoSch92 VascoSch92 linked an issue May 29, 2026 that may be closed by this pull request
@vercel
Copy link
Copy Markdown

vercel Bot commented May 29, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agent-canvas Ready Ready Preview, Comment May 29, 2026 12:13pm

Request Review

@VascoSch92 VascoSch92 requested a review from all-hands-bot May 29, 2026 12:13
Copy link
Copy Markdown
Contributor

all-hands-bot commented May 29, 2026

Review complete.

This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here.

Copy link
Copy Markdown
Contributor

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

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

Overall this is a clean, well-scoped fix. The PR correctly constraints condenser.max_size at the UI level to match the backend's ge=20 rule, and the approach — extracting a pure getNumericFieldError function and using the native bad-input flag for unparseable entries — is sound. Good test coverage and full i18n for all 15 locales. A few observations below.

This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation

Comment thread src/components/features/settings/sdk-settings/schema-field.tsx
Comment thread src/components/features/settings/sdk-settings/schema-field.tsx
Comment thread src/components/features/settings/sdk-settings/schema-field.tsx
@VascoSch92 VascoSch92 marked this pull request as ready for review May 29, 2026 12:36
@VascoSch92 VascoSch92 added the update-snapshots Intentional snapshot changes — CI diff check bypassed; new baselines uploaded on merge label May 29, 2026
github-actions Bot added a commit that referenced this pull request May 29, 2026
@github-actions
Copy link
Copy Markdown
Contributor

📸 Snapshot Test Report

Warning

Snapshot comparison step crashed (timeout, OOM, or runner error) — diff results below may be incomplete or absent.
Check the CI logs for the full error output (look for the "Run snapshot comparison" step).

Warning

One or more snapshot tests crashed during generation — some snapshots below may be incomplete.
Check the CI logs for the full error output (look for the "Generate current PR snapshots" step).

✅ 7 snapshots changed — acknowledged via the update-snapshots label. New baselines will be uploaded when this PR merges.

Category Count
🔴 Changed 7
🆕 New 0
✅ Unchanged 66
Total 73
🔴 Changed snapshots (7)

backends-extended

backend-after-switch

Expected (main) Actual (PR) Diff
expected actual diff

mcp-page — 5 snapshots

mcp-custom-server-1-editor-open

Expected (main) Actual (PR) Diff
expected actual diff

mcp-custom-server-editor

Expected (main) Actual (PR) Diff
expected actual diff

mcp-empty-installed

Expected (main) Actual (PR) Diff
expected actual diff

mcp-search-filtered

Expected (main) Actual (PR) Diff
expected actual diff

mcp-slack-install-1-marketplace

Expected (main) Actual (PR) Diff
expected actual diff

settings-page

settings-page

Expected (main) Actual (PR) Diff
expected actual diff
✅ Unchanged snapshots (66)

archived-conversation

  • conversation-panel-with-archived-badges
  • conversation-view-archived
  • conversation-view-sandbox-error

automations

  • automations-delete-modal
  • automations-list-active-inactive
  • automations-no-automations
  • automations-search-no-results

backends-extended

  • backend-add-blank-disabled
  • backend-add-cloud-advanced-open
  • backend-add-cloud-no-key-disabled
  • backend-add-cloud-with-key-enabled
  • backend-add-form-partially-filled
  • backend-add-invalid-url-disabled
  • backend-add-local-ready
  • backend-add-name-only-disabled
  • backend-add-two-column-layout
  • backend-add-whitespace-host-disabled
  • backend-cancel-nothing-saved
  • backend-dropdown-two-backends
  • backend-edit-prefilled
  • backend-manage-after-removal
  • backend-manage-two-listed
  • backend-remove-cancelled
  • backend-remove-confirmation
  • backend-switch-overlay

backends

  • backend-add-modal
  • backend-manage-modal
  • backend-selector-open

changes-tab

  • changes-deleted-file
  • changes-diff-viewer
  • changes-empty

collapsible-thinking

  • reasoning-content-collapsed
  • reasoning-content-expanded
  • think-action-collapsed
  • think-action-expanded

mcp-page

  • mcp-custom-server-2-url-filled
  • mcp-custom-server-3-all-filled
  • mcp-custom-server-4-installed
  • mcp-slack-install-2-modal
  • mcp-slack-install-3-filled
  • mcp-slack-install-4-installed

onboarding

  • onboarding-step-0-choose-agent
  • onboarding-step-1-check-backend
  • onboarding-step-2-setup-llm
  • onboarding-step-3-say-hello

projects-workspace-browser

  • projects-workspace-browser

settings-page

  • add-backend-modal
  • analytics-consent-modal
  • home-screen
  • settings-app-page

settings-secrets

  • secrets-add-form-filled
  • secrets-add-form
  • secrets-after-save
  • secrets-delete-confirm
  • secrets-list

settings-verification

  • condenser-settings
  • verification-settings-off
  • verification-settings-on

sidebar

  • sidebar-collapsed
  • sidebar-conversation-panel
  • sidebar-filter-menu

skills-page

  • skills-empty
  • skills-loaded
  • skills-no-match
  • skills-search-filtered
  • skills-type-filter

Generated by the Snapshot Tests workflow. This comment was created by an AI agent (OpenHands) on behalf of the repo maintainers.

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

Labels

update-snapshots Intentional snapshot changes — CI diff check bypassed; new baselines uploaded on merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Condenser Max Number of Events indicator can go under 0

2 participants