feat: [ENG-2490] add provider chip column to WebUI task list

[ncnthien]

Summary

• Problem:
• Why it matters:
• What changed:
• What did NOT change (scope boundary):

Type of change

• Bug fix
• New feature
• Refactor (no behavior change)
• Documentation
• Test
• Chore (build, dependencies, CI)

Scope (select all touched areas)

• TUI / REPL
• Agent / Tools
• LLM Providers
• Server / Daemon
• Shared (constants, types, transport events)
• CLI Commands (oclif)
• Hub / Connectors
• Cloud Sync
• CI/CD / Infra

Linked issues

• Closes #
• Related #

Root cause (bug fixes only, otherwise write N/A)

• Root cause:
• Why this was not caught earlier:

Test plan

• Coverage added:
  • Unit test
  • Integration test
  • Manual verification only
• Test file(s):
• Key scenario(s) covered:

User-visible changes

List user-visible changes (including defaults, config, or CLI output).
If none, write None.

Evidence

Attach at least one:

• Failing test/log before + passing after
• Trace/log snippets
• Screenshot/recording

Checklist

• Tests added or updated and passing (npm test)
• Lint passes (npm run lint)
• Type check passes (npm run typecheck)
• Build succeeds (npm run build)
• Commits follow Conventional Commits format
• Documentation updated (if applicable)
• No breaking changes (or clearly documented above)
• Branch is up to date with main

Risks and mitigations

List real risks for this PR. If none, write None.

• Risk:
  • Mitigation:

[github-actions]

Claude finished @ncnthien's task in 1m 42s —— View job

---

PR Review: feat: [ENG-2490] add provider chip column to WebUI task list

Tasks

• Gather context and read changed files
• Review task-list-table.tsx
• Review format-provider-model.ts
• Review format-provider-model.test.ts
• Post final review

---

Summary

Overall score: 4 / 5

Clean, well-scoped addition. The utility function is minimal and correct, the component is a natural fit alongside TypeBadge, and the colSpan is updated correctly (8 → 9). Tests cover the meaningful edge cases. A few minor points below.

---

What's good

• formatProviderModel returns undefined (not null) — consistent with the codebase convention.
• ProviderChip gracefully renders nothing when there's no label — no empty badge flash.
• title={label} on the badge gives a tooltip for truncated values — good accessibility touch.
• w-44 (176 px) is a sensible fixed width for <provider>:<model> pairs without blowing up the table layout.
• Test file covers the main contract cases: no provider, provider-only, full pair, model-only, empty strings.

---

Issues

1. Missing edge-case test — ('validProvider', '') → 'openai'
(inline comment on format-provider-model.test.ts lines 23–27)

The empty-string block covers ('', '') and ('', 'gpt-5-pro') but not ('openai', ''). The implementation returns just 'openai' in that case (because !'' is truthy), which is the right behaviour — but it's unverified. Fix this →

2. Implicit empty-string handling in formatProviderModel
(inline comment on format-provider-model.ts line 2)

!provider silently treats '' as missing via falsy coercion. This is intentional and tested, but a future reader might not realise it. No change required — just be aware if TaskListItem ever sends "" from the wire.

3. Long className string in ProviderChip badge
(inline comment on task-list-table.tsx line 204)

Minor readability nitpick — the class string is 130+ characters on one line. No functional issue; only matters if the formatter enforces a print-width rule.

---

No concerns about

• Security: no user input reaches the DOM unescaped; React handles escaping.
• Type safety: model? and provider? are both string | undefined in TaskListItem — the optional props in ProviderChip match exactly.
• Accessibility: title tooltip is present.
• Column count: colSpan={9} correctly accounts for the new Provider column.

[github-actions]

nitpick: The falsy check !provider silently treats an empty string '' as missing, which is the intended behavior (covered by the test on line 23). However, the logic is slightly implicit. If there's ever a scenario where an empty string slips through from the wire type, the function already handles it correctly — just worth knowing this is intentional falsy-coercion, not an explicit === undefined guard.

[github-actions]

suggestion: The empty-string block covers ('', '') and ('', 'gpt-5-pro') but misses the symmetric case: ('openai', ''). Based on the implementation !'' is truthy, so it would return just 'openai' — same as the "byterover internal" case. Adding one more assertion here would make the contract explicit:

Suggested change

	it('treats empty strings as missing', () => {
	expect(formatProviderModel('', '')).to.equal(undefined)
	expect(formatProviderModel('', 'gpt-5-pro')).to.equal(undefined)
	})
	})
	it('treats empty strings as missing', () => {
	expect(formatProviderModel('', '')).to.equal(undefined)
	expect(formatProviderModel('', 'gpt-5-pro')).to.equal(undefined)
	expect(formatProviderModel('openai', '')).to.equal('openai')
	})

[github-actions]

nitpick: The className string is quite wide (130+ chars). The existing TypeBadge keeps its className on one line too, so this is stylistically consistent — but if a linter or formatter run flags it, splitting into a cn(...) call would be the natural fix:

Suggested change

	<Badge className="text-muted-foreground mono max-w-full truncate text-[10px] tracking-wider" title={label} variant="outline">
	<Badge
	className={cn('text-muted-foreground mono max-w-full truncate text-[10px] tracking-wider')}
	variant="outline"
	>
	{label}
	</Badge>

(or simply wrap into a multi-line JSX if the formatter enforces it)