fix(tui): restore column width auto-sizing after requested model fields#597
Merged
fix(tui): restore column width auto-sizing after requested model fields#597
Conversation
d2d671f introduced colRequestedModel and colRequestedProvider with three bugs that broke auto-sizing for existing users: 1. allHeaders array omitted header text for the new columns (indices 12-13 defaulted to ""), so they rendered with no header 2. Minimum widths (15, 18) were larger than the header text (9, 12), stealing extra space from flex columns even when content was empty 3. migrateColumnConfig did not backfill new default-hidden columns into existing hidden_columns configs, so users with saved column prefs saw two empty columns consuming 33+ chars of table width Fix by adding the missing headers, correcting minimums to match header widths, and backfilling default-hidden columns during config migration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add three invariant tests that catch the class of bug introduced in d2d671f where new columns were added to the enum but metadata structures were not fully updated: - TestColumnMetadataComplete: every toggleable column must have entries in columnNames and columnConfigNames - TestAllColumnsVisibleHeadersPresent: renders with all columns visible and verifies each header appears (catches missing allHeaders entries) - TestQueueRenderWithStaleHiddenConfig: simulates a pre-upgrade config and verifies migration backfills new default-hidden columns so flex columns retain usable widths All three tests were verified to fail against the pre-fix code and pass after the fix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address review findings from #11439 and #11445: - Add ColumnConfigVersion to Config so the backfill migration runs exactly once. A user who later unhides columns won't get them re-hidden on next startup (version >= 1 skips the migration). - Iterate toggleableColumns (a slice) instead of defaultHiddenColumns (a map) for deterministic append order. - Remove the else-if branch that bumped version for configs without explicit hidden_columns — those use parseHiddenColumns(nil) defaults and need no migration. - Add TestQueueRenderPostMigrationUserChoice to verify post-migration configs are not re-migrated. - Add "version 1 config not re-migrated" case to TestMigrateColumnConfig. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
saveColumnOptions now writes ColumnConfigVersion = 1 alongside the user's column preferences. Without this, a user starting from a version-0 config (sentinel or nil hidden_columns) could save custom column choices, but the next startup would treat the saved config as stale and backfill default-hidden columns — undoing the user's choice. Add TestSaveColumnOptionStampsVersion to verify the save-then-reload cycle preserves preferences. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
roborev: Combined Review (
|
- TestSaveColumnOptionsWritesVersion: exercises the real saveColumnOptions → LoadGlobal/SaveGlobal path via ROBOREV_DATA_DIR, verifying ColumnConfigVersion = 1 is persisted and survives reload - TestMigratePreV1ConfigWithVisibleNewColumns: documents that the v1 migration backfills default-hidden columns for unversioned configs, and verifies the version stamp prevents re-migration on second run Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
roborev: Combined Review (
|
The backfill loop was iterating all defaultHiddenColumns, which includes session_id — a column that existed before d2d671f. A user with hidden_columns = ["branch"] may have intentionally shown session_id; the migration should not override that choice. Scope the backfill to only colRequestedModel and colRequestedProvider (the columns actually introduced in d2d671f). Add a test case verifying session_id visibility is preserved during migration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
roborev: Combined Review (
|
mariusvniekerk
approved these changes
Mar 30, 2026
Assert the reloaded HiddenColumns value matches the user's selection, not just the version stamp. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
allHeaders, excessive minimum widths (15/18 instead of 9/12), and no migration backfill for new default-hidden columnsColumnConfigVersion) so the backfill runs exactly once and does not re-hide columns a user later unhidesColumnConfigVersioninsaveColumnOptionsso user-saved preferences from unversioned configs are not re-migrated on next startupTestColumnMetadataComplete,TestAllColumnsVisibleHeadersPresent,TestQueueRenderWithStaleHiddenConfig) that catch this class of bug when new columns are added🤖 Generated with Claude Code