Skip to content

MAINT: Refactor converter construction into ConverterClassRegistry #1963

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rlundeen2 wants to merge 5 commits into
base: main
Choose a base branch
from
Open

MAINT: Refactor converter construction into ConverterClassRegistry #1963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cf76325
Refactor converter construction into ConverterClassRegistry
rlundeen2 Jun 9, 2026
2dbbef0
Fix ty: fall back to creation_time for in-progress updated_at
rlundeen2 Jun 9, 2026
b49d2a8
Merge branch 'main' into rlundeen2/converter-registry-refactor
rlundeen2 Jun 9, 2026
2e6f5ec
MAINT: Unify converter registry into buildable ContainerRegistry
rlundeen2 Jun 12, 2026
2336b49
MAINT: Address PR review nits on converter registry
rlundeen2 Jun 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions frontend/src/components/Chat/ConverterPanel.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,29 @@ describe('ConverterPanel loading', () => {
renderPanel()
await waitFor(() => expect(screen.getByTestId('converter-panel-empty')).toBeInTheDocument())
})

it('hides base/helper converters that should not be offered', async () => {
const catalogWithHidden = {
items: [
...MOCK_CATALOG.items,
{
converter_type: 'SelectiveTextConverter',
supported_input_types: ['text'],
supported_output_types: ['text'],
parameters: [],
is_llm_based: false,
description: 'Base/helper converter.',
},
],
}
mockedConvertersApi.listConverterCatalog.mockResolvedValueOnce(catalogWithHidden as ConverterCatalogResponse)
renderPanel()
await waitForList()

fireEvent.click(getComboboxInput())
await waitFor(() => expect(screen.getByTestId('converter-option-Base64Converter')).toBeInTheDocument())
expect(screen.queryByTestId('converter-option-SelectiveTextConverter')).not.toBeInTheDocument()
})
})

// ─── Close button ────────────────────────────────────────────────
Expand Down
6 changes: 5 additions & 1 deletion frontend/src/components/Chat/ConverterPanel/ConverterPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ const PIECE_TYPE_LABELS: Record<string, string> = {
video: 'Video',
}

// Converter classes the backend can build but that aren't useful to offer in the
// picker (base/helper classes).
const HIDDEN_CONVERTER_TYPES = new Set<string>(['SelectiveTextConverter'])

@jsong468 jsong468 Jun 11, 2026

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.

Is this something we want to eventually offer in the gui? also right now it seems like this is hard-coded here and in the tests. If this is a set we expect to expand or change (probably not really the case), there could be an attribute like gui_visible in the metadata.

@rlundeen2 rlundeen2 Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Rich's response (copilot generated): Good idea, but we're deliberately keeping visibility out of the registry metadata. The registry/catalog intentionally surfaces every constructible converter so agents and attack strategies can build anything (e.g. SelectiveTextConverter is hidden from the picker but must stay discoverable/buildable). Which entries to show is a presentation concern owned by the frontend, so the hide-list stays here in ConverterPanel.tsx — and there's a test (test_metadata_has_no_catalog_visible_field) that enforces metadata staying presentation-free. If this list grows or needs to vary per surface, we'd add a presentation-layer config rather than a gui_visible field on the converter class metadata.


interface ConverterPanelProps {
onClose: () => void
previewText?: string
Expand Down Expand Up @@ -51,7 +55,7 @@ export default function ConverterPanel({ onClose, previewText = '', attachmentDa

try {
const response = await convertersApi.listConverterCatalog()
setConverters(response.items)
setConverters(response.items.filter((c) => !HIDDEN_CONVERTER_TYPES.has(c.converter_type)))
} catch (err) {
setConverters([])
setSelectedConverterType('')
Expand Down
Loading