diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 4f6323e4b9..43274050bf 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -14922,6 +14922,19 @@ const docTemplate = `{ } } }, + "codersdk.AgentDisplayMode": { + "type": "string", + "enum": [ + "auto", + "always_expanded", + "always_collapsed" + ], + "x-enum-varnames": [ + "AgentDisplayModeAuto", + "AgentDisplayModeAlwaysExpanded", + "AgentDisplayModeAlwaysCollapsed" + ] + }, "codersdk.AgentScriptTiming": { "type": "object", "properties": { @@ -23385,6 +23398,9 @@ const docTemplate = `{ "codersdk.UpdateUserPreferenceSettingsRequest": { "type": "object", "properties": { + "code_diff_display_mode": { + "$ref": "#/definitions/codersdk.AgentDisplayMode" + }, "task_notification_alert_dismissed": { "type": "boolean" }, @@ -23855,6 +23871,9 @@ const docTemplate = `{ "codersdk.UserPreferenceSettings": { "type": "object", "properties": { + "code_diff_display_mode": { + "$ref": "#/definitions/codersdk.AgentDisplayMode" + }, "task_notification_alert_dismissed": { "type": "boolean" }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 77c0817ec1..7714a2662e 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -13388,6 +13388,15 @@ } } }, + "codersdk.AgentDisplayMode": { + "type": "string", + "enum": ["auto", "always_expanded", "always_collapsed"], + "x-enum-varnames": [ + "AgentDisplayModeAuto", + "AgentDisplayModeAlwaysExpanded", + "AgentDisplayModeAlwaysCollapsed" + ] + }, "codersdk.AgentScriptTiming": { "type": "object", "properties": { @@ -21529,6 +21538,9 @@ "codersdk.UpdateUserPreferenceSettingsRequest": { "type": "object", "properties": { + "code_diff_display_mode": { + "$ref": "#/definitions/codersdk.AgentDisplayMode" + }, "task_notification_alert_dismissed": { "type": "boolean" }, @@ -21970,6 +21982,9 @@ "codersdk.UserPreferenceSettings": { "type": "object", "properties": { + "code_diff_display_mode": { + "$ref": "#/definitions/codersdk.AgentDisplayMode" + }, "task_notification_alert_dismissed": { "type": "boolean" }, diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index eaea49fb9f..41334f3823 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -4408,6 +4408,17 @@ func (q *querier) GetUserChatSpendInPeriod(ctx context.Context, arg database.Get return q.db.GetUserChatSpendInPeriod(ctx, arg) } +func (q *querier) GetUserCodeDiffDisplayMode(ctx context.Context, userID uuid.UUID) (string, error) { + user, err := q.db.GetUserByID(ctx, userID) + if err != nil { + return "", err + } + if err := q.authorizeContext(ctx, policy.ActionReadPersonal, user); err != nil { + return "", err + } + return q.db.GetUserCodeDiffDisplayMode(ctx, userID) +} + func (q *querier) GetUserCount(ctx context.Context, includeSystem bool) (int64, error) { if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil { return 0, err @@ -7024,6 +7035,17 @@ func (q *querier) UpdateUserChatProviderKey(ctx context.Context, arg database.Up return q.db.UpdateUserChatProviderKey(ctx, arg) } +func (q *querier) UpdateUserCodeDiffDisplayMode(ctx context.Context, arg database.UpdateUserCodeDiffDisplayModeParams) (string, error) { + user, err := q.db.GetUserByID(ctx, arg.UserID) + if err != nil { + return "", err + } + if err := q.authorizeContext(ctx, policy.ActionUpdatePersonal, user); err != nil { + return "", err + } + return q.db.UpdateUserCodeDiffDisplayMode(ctx, arg) +} + func (q *querier) UpdateUserDeletedByID(ctx context.Context, id uuid.UUID) error { return deleteQ(q.log, q.auth, q.db.GetUserByID, q.db.UpdateUserDeletedByID)(ctx, id) } diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 53c0ed35bd..6a749a2bb5 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -2829,6 +2829,19 @@ func (s *MethodTestSuite) TestUser() { dbm.EXPECT().UpdateUserThinkingDisplayMode(gomock.Any(), arg).Return("always_expanded", nil).AnyTimes() check.Args(arg).Asserts(u, policy.ActionUpdatePersonal).Returns("always_expanded") })) + s.Run("GetUserCodeDiffDisplayMode", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + u := testutil.Fake(s.T(), faker, database.User{}) + dbm.EXPECT().GetUserByID(gomock.Any(), u.ID).Return(u, nil).AnyTimes() + dbm.EXPECT().GetUserCodeDiffDisplayMode(gomock.Any(), u.ID).Return("auto", nil).AnyTimes() + check.Args(u.ID).Asserts(u, policy.ActionReadPersonal).Returns("auto") + })) + s.Run("UpdateUserCodeDiffDisplayMode", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + u := testutil.Fake(s.T(), faker, database.User{}) + arg := database.UpdateUserCodeDiffDisplayModeParams{UserID: u.ID, CodeDiffDisplayMode: "always_collapsed"} + dbm.EXPECT().GetUserByID(gomock.Any(), u.ID).Return(u, nil).AnyTimes() + dbm.EXPECT().UpdateUserCodeDiffDisplayMode(gomock.Any(), arg).Return("always_collapsed", nil).AnyTimes() + check.Args(arg).Asserts(u, policy.ActionUpdatePersonal).Returns("always_collapsed") + })) s.Run("ListUserChatCompactionThresholds", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { u := testutil.Fake(s.T(), faker, database.User{}) uc := database.UserConfig{UserID: u.ID, Key: codersdk.ChatCompactionThresholdKeyPrefix + "00000000-0000-0000-0000-000000000001", Value: "75"} diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 7930faabf5..e7f4378427 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -2848,6 +2848,14 @@ func (m queryMetricsStore) GetUserChatSpendInPeriod(ctx context.Context, arg dat return r0, r1 } +func (m queryMetricsStore) GetUserCodeDiffDisplayMode(ctx context.Context, userID uuid.UUID) (string, error) { + start := time.Now() + r0, r1 := m.s.GetUserCodeDiffDisplayMode(ctx, userID) + m.queryLatencies.WithLabelValues("GetUserCodeDiffDisplayMode").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "GetUserCodeDiffDisplayMode").Inc() + return r0, r1 +} + func (m queryMetricsStore) GetUserCount(ctx context.Context, includeSystem bool) (int64, error) { start := time.Now() r0, r1 := m.s.GetUserCount(ctx, includeSystem) @@ -5008,6 +5016,14 @@ func (m queryMetricsStore) UpdateUserChatProviderKey(ctx context.Context, arg da return r0, r1 } +func (m queryMetricsStore) UpdateUserCodeDiffDisplayMode(ctx context.Context, arg database.UpdateUserCodeDiffDisplayModeParams) (string, error) { + start := time.Now() + r0, r1 := m.s.UpdateUserCodeDiffDisplayMode(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateUserCodeDiffDisplayMode").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "UpdateUserCodeDiffDisplayMode").Inc() + return r0, r1 +} + func (m queryMetricsStore) UpdateUserDeletedByID(ctx context.Context, id uuid.UUID) error { start := time.Now() r0 := m.s.UpdateUserDeletedByID(ctx, id) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index e5be0fa48b..d84681026e 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -5327,6 +5327,21 @@ func (mr *MockStoreMockRecorder) GetUserChatSpendInPeriod(ctx, arg any) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserChatSpendInPeriod", reflect.TypeOf((*MockStore)(nil).GetUserChatSpendInPeriod), ctx, arg) } +// GetUserCodeDiffDisplayMode mocks base method. +func (m *MockStore) GetUserCodeDiffDisplayMode(ctx context.Context, userID uuid.UUID) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserCodeDiffDisplayMode", ctx, userID) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserCodeDiffDisplayMode indicates an expected call of GetUserCodeDiffDisplayMode. +func (mr *MockStoreMockRecorder) GetUserCodeDiffDisplayMode(ctx, userID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserCodeDiffDisplayMode", reflect.TypeOf((*MockStore)(nil).GetUserCodeDiffDisplayMode), ctx, userID) +} + // GetUserCount mocks base method. func (m *MockStore) GetUserCount(ctx context.Context, includeSystem bool) (int64, error) { m.ctrl.T.Helper() @@ -9441,6 +9456,21 @@ func (mr *MockStoreMockRecorder) UpdateUserChatProviderKey(ctx, arg any) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateUserChatProviderKey", reflect.TypeOf((*MockStore)(nil).UpdateUserChatProviderKey), ctx, arg) } +// UpdateUserCodeDiffDisplayMode mocks base method. +func (m *MockStore) UpdateUserCodeDiffDisplayMode(ctx context.Context, arg database.UpdateUserCodeDiffDisplayModeParams) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateUserCodeDiffDisplayMode", ctx, arg) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateUserCodeDiffDisplayMode indicates an expected call of UpdateUserCodeDiffDisplayMode. +func (mr *MockStoreMockRecorder) UpdateUserCodeDiffDisplayMode(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateUserCodeDiffDisplayMode", reflect.TypeOf((*MockStore)(nil).UpdateUserCodeDiffDisplayMode), ctx, arg) +} + // UpdateUserDeletedByID mocks base method. func (m *MockStore) UpdateUserDeletedByID(ctx context.Context, id uuid.UUID) error { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 5d025f4b25..44273cfc6b 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -709,6 +709,7 @@ type sqlcQuerier interface { // returned (global behavior). Otherwise only spend within the // specified organization is included. GetUserChatSpendInPeriod(ctx context.Context, arg GetUserChatSpendInPeriodParams) (int64, error) + GetUserCodeDiffDisplayMode(ctx context.Context, userID uuid.UUID) (string, error) GetUserCount(ctx context.Context, includeSystem bool) (int64, error) // Returns the minimum (most restrictive) group limit for a user. // Returns -1 if no group limits match the specified scope. @@ -1193,6 +1194,7 @@ type sqlcQuerier interface { UpdateUserChatCompactionThreshold(ctx context.Context, arg UpdateUserChatCompactionThresholdParams) (UserConfig, error) UpdateUserChatCustomPrompt(ctx context.Context, arg UpdateUserChatCustomPromptParams) (UserConfig, error) UpdateUserChatProviderKey(ctx context.Context, arg UpdateUserChatProviderKeyParams) (UserChatProviderKey, error) + UpdateUserCodeDiffDisplayMode(ctx context.Context, arg UpdateUserCodeDiffDisplayModeParams) (string, error) UpdateUserDeletedByID(ctx context.Context, id uuid.UUID) error UpdateUserGithubComUserID(ctx context.Context, arg UpdateUserGithubComUserIDParams) error UpdateUserHashedOneTimePasscode(ctx context.Context, arg UpdateUserHashedOneTimePasscodeParams) error diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index e6cc1a361f..6979ea3037 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -25703,6 +25703,23 @@ func (q *sqlQuerier) GetUserChatPersonalModelOverride(ctx context.Context, arg G return personal_model_override, err } +const getUserCodeDiffDisplayMode = `-- name: GetUserCodeDiffDisplayMode :one +SELECT + value AS code_diff_display_mode +FROM + user_configs +WHERE + user_id = $1 + AND key = 'preference_code_diff_display_mode' +` + +func (q *sqlQuerier) GetUserCodeDiffDisplayMode(ctx context.Context, userID uuid.UUID) (string, error) { + row := q.db.QueryRowContext(ctx, getUserCodeDiffDisplayMode, userID) + var code_diff_display_mode string + err := row.Scan(&code_diff_display_mode) + return code_diff_display_mode, err +} + const getUserCount = `-- name: GetUserCount :one SELECT COUNT(*) @@ -26301,6 +26318,33 @@ func (q *sqlQuerier) UpdateUserChatCustomPrompt(ctx context.Context, arg UpdateU return i, err } +const updateUserCodeDiffDisplayMode = `-- name: UpdateUserCodeDiffDisplayMode :one +INSERT INTO + user_configs (user_id, key, value) +VALUES + ($1, 'preference_code_diff_display_mode', $2::text) +ON CONFLICT + ON CONSTRAINT user_configs_pkey +DO UPDATE +SET + value = $2 +WHERE user_configs.user_id = $1 + AND user_configs.key = 'preference_code_diff_display_mode' +RETURNING value AS code_diff_display_mode +` + +type UpdateUserCodeDiffDisplayModeParams struct { + UserID uuid.UUID `db:"user_id" json:"user_id"` + CodeDiffDisplayMode string `db:"code_diff_display_mode" json:"code_diff_display_mode"` +} + +func (q *sqlQuerier) UpdateUserCodeDiffDisplayMode(ctx context.Context, arg UpdateUserCodeDiffDisplayModeParams) (string, error) { + row := q.db.QueryRowContext(ctx, updateUserCodeDiffDisplayMode, arg.UserID, arg.CodeDiffDisplayMode) + var code_diff_display_mode string + err := row.Scan(&code_diff_display_mode) + return code_diff_display_mode, err +} + const updateUserDeletedByID = `-- name: UpdateUserDeletedByID :exec UPDATE users diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index a76c8361a5..e4cff00cc7 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -303,6 +303,30 @@ WHERE user_configs.user_id = @user_id AND user_configs.key = 'preference_thinking_display_mode' RETURNING value AS thinking_display_mode; + +-- name: GetUserCodeDiffDisplayMode :one +SELECT + value AS code_diff_display_mode +FROM + user_configs +WHERE + user_id = @user_id + AND key = 'preference_code_diff_display_mode'; + +-- name: UpdateUserCodeDiffDisplayMode :one +INSERT INTO + user_configs (user_id, key, value) +VALUES + (@user_id, 'preference_code_diff_display_mode', @code_diff_display_mode::text) +ON CONFLICT + ON CONSTRAINT user_configs_pkey +DO UPDATE +SET + value = @code_diff_display_mode +WHERE user_configs.user_id = @user_id + AND user_configs.key = 'preference_code_diff_display_mode' +RETURNING value AS code_diff_display_mode; + -- name: UpdateUserRoles :one UPDATE users diff --git a/coderd/users.go b/coderd/users.go index c207e2620f..fd6ff00b0f 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -1248,9 +1248,19 @@ func (api *API) userPreferenceSettings(rw http.ResponseWriter, r *http.Request) return } + codeDiffMode, err := api.Database.GetUserCodeDiffDisplayMode(ctx, user.ID) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Error reading user preference settings.", + Detail: err.Error(), + }) + return + } + httpapi.Write(ctx, rw, http.StatusOK, codersdk.UserPreferenceSettings{ TaskNotificationAlertDismissed: taskAlertDismissed, ThinkingDisplayMode: sanitizeThinkingDisplayMode(thinkingMode), + CodeDiffDisplayMode: sanitizeAgentDisplayMode(codeDiffMode), }) } @@ -1280,69 +1290,120 @@ func (api *API) putUserPreferenceSettings(rw http.ResponseWriter, r *http.Reques httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "Invalid thinking display mode.", Validations: []codersdk.ValidationError{ - {Field: "thinking_display_mode", Detail: "must be one of: auto, preview, always_expanded, always_collapsed"}, + {Field: "thinking_display_mode", Detail: thinkingDisplayModeValidationDetail}, }, }) return } - var err error - - var updatedTaskAlertDismissed bool - if params.TaskNotificationAlertDismissed != nil { - updatedTaskAlertDismissed, err = api.Database.UpdateUserTaskNotificationAlertDismissed(ctx, database.UpdateUserTaskNotificationAlertDismissedParams{ - UserID: user.ID, - TaskNotificationAlertDismissed: *params.TaskNotificationAlertDismissed, + if params.CodeDiffDisplayMode != "" && + !slices.Contains(codersdk.ValidAgentDisplayModes, params.CodeDiffDisplayMode) { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Invalid code diff display mode.", + Validations: []codersdk.ValidationError{ + {Field: "code_diff_display_mode", Detail: agentDisplayModeValidationDetail}, + }, }) - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error updating user task notification alert dismissed.", - Detail: err.Error(), - }) - return - } - } else { - updatedTaskAlertDismissed, err = api.Database.GetUserTaskNotificationAlertDismissed(ctx, user.ID) - if err != nil && !errors.Is(err, sql.ErrNoRows) { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Error reading task notification alert dismissed.", - Detail: err.Error(), + return + } + var settings codersdk.UserPreferenceSettings + err := api.Database.InTx(func(tx database.Store) error { + var err error + if params.TaskNotificationAlertDismissed != nil { + settings.TaskNotificationAlertDismissed, err = tx.UpdateUserTaskNotificationAlertDismissed(ctx, database.UpdateUserTaskNotificationAlertDismissedParams{ + UserID: user.ID, + TaskNotificationAlertDismissed: *params.TaskNotificationAlertDismissed, }) + if err != nil { + return newUserPreferenceSettingsAPIError("Internal error updating user task notification alert dismissed.", err) + } + } else { + settings.TaskNotificationAlertDismissed, err = tx.GetUserTaskNotificationAlertDismissed(ctx, user.ID) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return newUserPreferenceSettingsAPIError("Error reading task notification alert dismissed.", err) + } + } + + if params.ThinkingDisplayMode != "" { + updated, err := tx.UpdateUserThinkingDisplayMode(ctx, database.UpdateUserThinkingDisplayModeParams{ + UserID: user.ID, + ThinkingDisplayMode: string(params.ThinkingDisplayMode), + }) + if err != nil { + return newUserPreferenceSettingsAPIError("Internal error updating thinking display mode.", err) + } + settings.ThinkingDisplayMode = sanitizeThinkingDisplayMode(updated) + } else { + stored, err := tx.GetUserThinkingDisplayMode(ctx, user.ID) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return newUserPreferenceSettingsAPIError("Error reading thinking display mode.", err) + } + settings.ThinkingDisplayMode = sanitizeThinkingDisplayMode(stored) + } + + if params.CodeDiffDisplayMode != "" { + updated, err := tx.UpdateUserCodeDiffDisplayMode(ctx, database.UpdateUserCodeDiffDisplayModeParams{ + UserID: user.ID, + CodeDiffDisplayMode: string(params.CodeDiffDisplayMode), + }) + if err != nil { + return newUserPreferenceSettingsAPIError("Internal error updating code diff display mode.", err) + } + settings.CodeDiffDisplayMode = sanitizeAgentDisplayMode(updated) + } else { + stored, err := tx.GetUserCodeDiffDisplayMode(ctx, user.ID) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return newUserPreferenceSettingsAPIError("Error reading code diff display mode.", err) + } + settings.CodeDiffDisplayMode = sanitizeAgentDisplayMode(stored) + } + return nil + }, database.DefaultTXOptions().WithID("user_preference_settings")) + if err != nil { + var apiErr userPreferenceSettingsAPIError + if errors.As(err, &apiErr) { + httpapi.Write(ctx, rw, apiErr.statusCode, apiErr.response) return } + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error updating user preference settings.", + Detail: err.Error(), + }) + return } - var resolvedThinkingMode codersdk.ThinkingDisplayMode - if params.ThinkingDisplayMode != "" { - updated, err := api.Database.UpdateUserThinkingDisplayMode(ctx, database.UpdateUserThinkingDisplayModeParams{ - UserID: user.ID, - ThinkingDisplayMode: string(params.ThinkingDisplayMode), - }) - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error updating thinking display mode.", - Detail: err.Error(), - }) - return - } - resolvedThinkingMode = codersdk.ThinkingDisplayMode(updated) - } else { - stored, err := api.Database.GetUserThinkingDisplayMode(ctx, user.ID) - if err != nil && !errors.Is(err, sql.ErrNoRows) { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Error reading thinking display mode.", - Detail: err.Error(), - }) - return - } - resolvedThinkingMode = sanitizeThinkingDisplayMode(stored) - } - - httpapi.Write(ctx, rw, http.StatusOK, codersdk.UserPreferenceSettings{ - TaskNotificationAlertDismissed: updatedTaskAlertDismissed, - ThinkingDisplayMode: resolvedThinkingMode, - }) + httpapi.Write(ctx, rw, http.StatusOK, settings) } +type userPreferenceSettingsAPIError struct { + statusCode int + response codersdk.Response + err error +} + +func newUserPreferenceSettingsAPIError(message string, err error) userPreferenceSettingsAPIError { + return userPreferenceSettingsAPIError{ + statusCode: http.StatusInternalServerError, + response: codersdk.Response{ + Message: message, + Detail: err.Error(), + }, + err: err, + } +} + +func (e userPreferenceSettingsAPIError) Error() string { + return fmt.Sprintf("%s: %s", e.response.Message, e.err) +} + +func (e userPreferenceSettingsAPIError) Unwrap() error { + return e.err +} + +const ( + thinkingDisplayModeValidationDetail = "must be one of: auto, preview, always_expanded, always_collapsed" + agentDisplayModeValidationDetail = "must be one of: auto, always_expanded, always_collapsed" +) + func sanitizeThinkingDisplayMode(raw string) codersdk.ThinkingDisplayMode { mode := codersdk.ThinkingDisplayMode(raw) if slices.Contains(codersdk.ValidThinkingDisplayModes, mode) { @@ -1351,6 +1412,14 @@ func sanitizeThinkingDisplayMode(raw string) codersdk.ThinkingDisplayMode { return codersdk.ThinkingDisplayModeAuto } +func sanitizeAgentDisplayMode(raw string) codersdk.AgentDisplayMode { + mode := codersdk.AgentDisplayMode(raw) + if slices.Contains(codersdk.ValidAgentDisplayModes, mode) { + return mode + } + return codersdk.AgentDisplayModeAuto +} + func isValidFontName(font codersdk.TerminalFontName) bool { return slices.Contains(codersdk.TerminalFontNames, font) } diff --git a/coderd/users_test.go b/coderd/users_test.go index 8df7bf8297..1369ab22bc 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -1963,6 +1963,117 @@ func TestThinkingDisplayMode(t *testing.T) { }) } +func TestAgentDisplayModePreferences(t *testing.T) { + t.Parallel() + + adminClient := coderdtest.New(t, nil) + firstUser := coderdtest.CreateFirstUser(t, adminClient) + + requireValidationField := func(t *testing.T, err error, field string) { + t.Helper() + + var sdkErr *codersdk.Error + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) + require.Len(t, sdkErr.Validations, 1) + require.Equal(t, field, sdkErr.Validations[0].Field) + } + + t.Run("defaults to auto", func(t *testing.T) { + t.Parallel() + + client, _ := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + settings, err := client.GetUserPreferenceSettings(ctx, codersdk.Me) + require.NoError(t, err) + require.Equal(t, codersdk.AgentDisplayModeAuto, settings.CodeDiffDisplayMode) + }) + + t.Run("round-trips code diff display mode", func(t *testing.T) { + t.Parallel() + + client, _ := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + for _, mode := range []codersdk.AgentDisplayMode{ + codersdk.AgentDisplayModeAlwaysExpanded, + codersdk.AgentDisplayModeAlwaysCollapsed, + } { + updated, err := client.UpdateUserPreferenceSettings(ctx, codersdk.Me, codersdk.UpdateUserPreferenceSettingsRequest{ + CodeDiffDisplayMode: mode, + }) + require.NoError(t, err) + require.Equal(t, mode, updated.CodeDiffDisplayMode) + + settings, err := client.GetUserPreferenceSettings(ctx, codersdk.Me) + require.NoError(t, err) + require.Equal(t, mode, settings.CodeDiffDisplayMode) + } + }) + + t.Run("updates preserve stored display modes", func(t *testing.T) { + t.Parallel() + + client, _ := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + _, err := client.UpdateUserPreferenceSettings(ctx, codersdk.Me, codersdk.UpdateUserPreferenceSettingsRequest{ + ThinkingDisplayMode: codersdk.ThinkingDisplayModePreview, + CodeDiffDisplayMode: codersdk.AgentDisplayModeAlwaysExpanded, + }) + require.NoError(t, err) + + updated, err := client.UpdateUserPreferenceSettings(ctx, codersdk.Me, codersdk.UpdateUserPreferenceSettingsRequest{ + ThinkingDisplayMode: codersdk.ThinkingDisplayModeAlwaysExpanded, + }) + require.NoError(t, err) + require.Equal(t, codersdk.ThinkingDisplayModeAlwaysExpanded, updated.ThinkingDisplayMode) + require.Equal(t, codersdk.AgentDisplayModeAlwaysExpanded, updated.CodeDiffDisplayMode) + + settings, err := client.GetUserPreferenceSettings(ctx, codersdk.Me) + require.NoError(t, err) + require.Equal(t, codersdk.ThinkingDisplayModeAlwaysExpanded, settings.ThinkingDisplayMode) + require.Equal(t, codersdk.AgentDisplayModeAlwaysExpanded, settings.CodeDiffDisplayMode) + }) + + t.Run("rejects invalid code diff display mode", func(t *testing.T) { + t.Parallel() + + client, _ := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + for _, tt := range []struct { + name string + mode codersdk.AgentDisplayMode + }{ + { + name: "bogus", + mode: codersdk.AgentDisplayMode("bogus"), + }, + { + name: "thinking preview", + mode: codersdk.AgentDisplayMode(codersdk.ThinkingDisplayModePreview), + }, + } { + t.Run(tt.name, func(t *testing.T) { + _, err := client.UpdateUserPreferenceSettings(ctx, codersdk.Me, codersdk.UpdateUserPreferenceSettingsRequest{ + CodeDiffDisplayMode: tt.mode, + }) + requireValidationField(t, err, "code_diff_display_mode") + }) + } + }) +} + func TestWorkspacesByUser(t *testing.T) { t.Parallel() t.Run("Empty", func(t *testing.T) { diff --git a/codersdk/users.go b/codersdk/users.go index f90e10c763..c652be4766 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -265,11 +265,13 @@ type UpdateUserAppearanceSettingsRequest struct { type UserPreferenceSettings struct { TaskNotificationAlertDismissed bool `json:"task_notification_alert_dismissed"` ThinkingDisplayMode ThinkingDisplayMode `json:"thinking_display_mode"` + CodeDiffDisplayMode AgentDisplayMode `json:"code_diff_display_mode"` } type UpdateUserPreferenceSettingsRequest struct { TaskNotificationAlertDismissed *bool `json:"task_notification_alert_dismissed,omitempty"` ThinkingDisplayMode ThinkingDisplayMode `json:"thinking_display_mode,omitempty"` + CodeDiffDisplayMode AgentDisplayMode `json:"code_diff_display_mode,omitempty"` } type ThinkingDisplayMode string @@ -288,6 +290,20 @@ var ValidThinkingDisplayModes = []ThinkingDisplayMode{ ThinkingDisplayModeAlwaysCollapsed, } +type AgentDisplayMode string + +const ( + AgentDisplayModeAuto AgentDisplayMode = "auto" + AgentDisplayModeAlwaysExpanded AgentDisplayMode = "always_expanded" + AgentDisplayModeAlwaysCollapsed AgentDisplayMode = "always_collapsed" +) + +var ValidAgentDisplayModes = []AgentDisplayMode{ + AgentDisplayModeAuto, + AgentDisplayModeAlwaysExpanded, + AgentDisplayModeAlwaysCollapsed, +} + type UpdateUserPasswordRequest struct { OldPassword string `json:"old_password" validate:""` Password string `json:"password" validate:"required"` diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 976fba8186..c20d026091 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -1431,6 +1431,20 @@ | `workspace_agent_id` | string | false | | | | `workspace_agent_name` | string | false | | | +## codersdk.AgentDisplayMode + +```json +"auto" +``` + +### Properties + +#### Enumerated Values + +| Value(s) | +|-----------------------------------------------| +| `always_collapsed`, `always_expanded`, `auto` | + ## codersdk.AgentScriptTiming ```json @@ -12807,6 +12821,7 @@ Restarts will only happen on weekdays in this list on weeks which line up with W ```json { + "code_diff_display_mode": "auto", "task_notification_alert_dismissed": true, "thinking_display_mode": "auto" } @@ -12816,6 +12831,7 @@ Restarts will only happen on weekdays in this list on weeks which line up with W | Name | Type | Required | Restrictions | Description | |-------------------------------------|--------------------------------------------------------------|----------|--------------|-------------| +| `code_diff_display_mode` | [codersdk.AgentDisplayMode](#codersdkagentdisplaymode) | false | | | | `task_notification_alert_dismissed` | boolean | false | | | | `thinking_display_mode` | [codersdk.ThinkingDisplayMode](#codersdkthinkingdisplaymode) | false | | | @@ -13393,6 +13409,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| ```json { + "code_diff_display_mode": "auto", "task_notification_alert_dismissed": true, "thinking_display_mode": "auto" } @@ -13402,6 +13419,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| | Name | Type | Required | Restrictions | Description | |-------------------------------------|--------------------------------------------------------------|----------|--------------|-------------| +| `code_diff_display_mode` | [codersdk.AgentDisplayMode](#codersdkagentdisplaymode) | false | | | | `task_notification_alert_dismissed` | boolean | false | | | | `thinking_display_mode` | [codersdk.ThinkingDisplayMode](#codersdkthinkingdisplaymode) | false | | | diff --git a/docs/reference/api/users.md b/docs/reference/api/users.md index 9e62248763..3876efbc0a 100644 --- a/docs/reference/api/users.md +++ b/docs/reference/api/users.md @@ -1301,6 +1301,7 @@ curl -X GET http://coder-server:8080/api/v2/users/{user}/preferences \ ```json { + "code_diff_display_mode": "auto", "task_notification_alert_dismissed": true, "thinking_display_mode": "auto" } @@ -1332,6 +1333,7 @@ curl -X PUT http://coder-server:8080/api/v2/users/{user}/preferences \ ```json { + "code_diff_display_mode": "auto", "task_notification_alert_dismissed": true, "thinking_display_mode": "auto" } @@ -1350,6 +1352,7 @@ curl -X PUT http://coder-server:8080/api/v2/users/{user}/preferences \ ```json { + "code_diff_display_mode": "auto", "task_notification_alert_dismissed": true, "thinking_display_mode": "auto" } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 44c5c0ecf0..be124a19eb 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -839,6 +839,15 @@ export interface AgentConnectionTiming { readonly workspace_agent_name: string; } +// From codersdk/users.go +export type AgentDisplayMode = "always_collapsed" | "always_expanded" | "auto"; + +export const AgentDisplayModes: AgentDisplayMode[] = [ + "always_collapsed", + "always_expanded", + "auto", +]; + // From codersdk/workspacebuilds.go export interface AgentScriptTiming { readonly started_at: string; @@ -8349,6 +8358,7 @@ export interface UpdateUserPasswordRequest { export interface UpdateUserPreferenceSettingsRequest { readonly task_notification_alert_dismissed?: boolean; readonly thinking_display_mode?: ThinkingDisplayMode; + readonly code_diff_display_mode?: AgentDisplayMode; } // From codersdk/users.go @@ -8726,6 +8736,7 @@ export interface UserParameter { export interface UserPreferenceSettings { readonly task_notification_alert_dismissed: boolean; readonly thinking_display_mode: ThinkingDisplayMode; + readonly code_diff_display_mode: AgentDisplayMode; } // From codersdk/deployment.go diff --git a/site/src/pages/AgentsPage/AgentSettingsGeneralPageView.stories.tsx b/site/src/pages/AgentsPage/AgentSettingsGeneralPageView.stories.tsx index f76e38d3c1..283f306891 100644 --- a/site/src/pages/AgentsPage/AgentSettingsGeneralPageView.stories.tsx +++ b/site/src/pages/AgentsPage/AgentSettingsGeneralPageView.stories.tsx @@ -122,6 +122,27 @@ export const RendersChatLayoutSection: Story = { }, }; +export const RendersAgentDisplayModeSettings: Story = { + parameters: { + queries: [ + { + key: ["me", "preferences"], + data: { + task_notification_alert_dismissed: false, + thinking_display_mode: "auto" as const, + code_diff_display_mode: "auto" as const, + }, + }, + ], + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + + expect(await canvas.findByText("Thinking Display")).toBeVisible(); + expect(await canvas.findByText("Code Diff Display")).toBeVisible(); + }, +}; + export const ShowsChatDebugLoggingToggle: Story = { args: { userDebugLoggingData: { diff --git a/site/src/pages/AgentsPage/AgentSettingsGeneralPageView.tsx b/site/src/pages/AgentsPage/AgentSettingsGeneralPageView.tsx index deb8516735..359a6f0d91 100644 --- a/site/src/pages/AgentsPage/AgentSettingsGeneralPageView.tsx +++ b/site/src/pages/AgentsPage/AgentSettingsGeneralPageView.tsx @@ -2,9 +2,12 @@ import type { FC } from "react"; import type { UseMutateFunction } from "react-query"; import type * as TypesGen from "#/api/typesGenerated"; import { ChatFullWidthSettings } from "./components/ChatFullWidthSettings"; +import { + CodeDiffDisplaySettings, + ThinkingDisplaySettings, +} from "./components/DisplayModeSettings"; import { PersonalInstructionsSettings } from "./components/PersonalInstructionsSettings"; import { SectionHeader } from "./components/SectionHeader"; -import { ThinkingDisplaySettings } from "./components/ThinkingDisplaySettings"; import { UserChatDebugLoggingSettings } from "./components/UserChatDebugLoggingSettings"; export interface AgentSettingsGeneralPageViewProps { @@ -55,6 +58,7 @@ export const AgentSettingsGeneralPageView: FC< /> + { + const canvas = within(canvasElement); + expect(canvas.getByText(/Edited config\.ts/)).toBeVisible(); + expect(canvas.queryAllByTestId("edit-file-diff")).toHaveLength(0); + + const editFilesButton = canvas.getByRole("button", { + name: /Edited config\.ts/, + }); + expect(editFilesButton).toHaveAttribute("aria-expanded", "false"); + await userEvent.click(editFilesButton); + await waitFor(() => { + expect(canvas.getAllByTestId("edit-file-diff")).toHaveLength(1); + }); + }, +}; + /** * A completed thinking block with always_expanded mode should show * its content without user interaction. @@ -1765,6 +1838,7 @@ export const ThinkingBlockAlwaysExpanded: Story = { data: { task_notification_alert_dismissed: false, thinking_display_mode: "always_expanded" as const, + code_diff_display_mode: "auto" as const, }, }, ], @@ -1812,6 +1886,7 @@ export const ThinkingBlockAlwaysCollapsed: Story = { data: { task_notification_alert_dismissed: false, thinking_display_mode: "always_collapsed" as const, + code_diff_display_mode: "auto" as const, }, }, ], @@ -1860,6 +1935,7 @@ export const ThinkingBlockWithToolCall: Story = { data: { task_notification_alert_dismissed: false, thinking_display_mode: "always_collapsed" as const, + code_diff_display_mode: "auto" as const, }, }, ], @@ -1921,6 +1997,7 @@ export const ThinkingBlockAutoMode: Story = { data: { task_notification_alert_dismissed: false, thinking_display_mode: "auto" as const, + code_diff_display_mode: "auto" as const, }, }, ], @@ -1972,6 +2049,7 @@ export const ThinkingBlockPreviewMode: Story = { data: { task_notification_alert_dismissed: false, thinking_display_mode: "preview" as const, + code_diff_display_mode: "auto" as const, }, }, ], diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx index a1df150a7a..e38f65e8ce 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx @@ -275,6 +275,8 @@ export const BlockList: FC<{ const prefQuery = useQuery(preferenceSettings()); const thinkingDisplayMode: ThinkingDisplayMode = prefQuery.data?.thinking_display_mode || "auto"; + const codeDiffDisplayMode: TypesGen.AgentDisplayMode = + prefQuery.data?.code_diff_display_mode || "auto"; const toolByID = new Map(tools.map((tool) => [tool.id, tool])); @@ -365,6 +367,7 @@ export const BlockList: FC<{ name="Tool" status="running" isError={false} + codeDiffDisplayMode={codeDiffDisplayMode} subagentTitles={subagentTitles} subagentVariants={subagentVariants} subagentStatusOverrides={subagentStatusOverrides} @@ -381,6 +384,7 @@ export const BlockList: FC<{ status={tool.status} isError={tool.isError} killedBySignal={tool.killedBySignal} + codeDiffDisplayMode={codeDiffDisplayMode} subagentTitles={subagentTitles} subagentVariants={subagentVariants} showDesktopPreviews={showDesktopPreviews} @@ -436,6 +440,7 @@ export const BlockList: FC<{ status={tool.status} isError={tool.isError} killedBySignal={tool.killedBySignal} + codeDiffDisplayMode={codeDiffDisplayMode} subagentTitles={subagentTitles} subagentVariants={subagentVariants} showDesktopPreviews={showDesktopPreviews} diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/EditFilesTool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/EditFilesTool.tsx index 6de14b4c64..4d3f6d30cd 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/EditFilesTool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/EditFilesTool.tsx @@ -3,13 +3,19 @@ import type { FileDiffMetadata } from "@pierre/diffs"; import { FileDiff } from "@pierre/diffs/react"; import { LoaderIcon, TriangleAlertIcon } from "lucide-react"; import type React from "react"; +import type * as TypesGen from "#/api/typesGenerated"; import { ScrollArea } from "#/components/ScrollArea/ScrollArea"; import { Tooltip, TooltipContent, TooltipTrigger, } from "#/components/Tooltip/Tooltip"; -import { ToolCollapsible } from "./ToolCollapsible"; +import { + type AgentDisplayState, + isAgentDisplayFullyExpanded, + resolveAgentDisplayState, +} from "./displayMode"; +import { AgentDisplayModeToolCollapsible } from "./ToolCollapsible"; import { DIFFS_FONT_STYLE, type EditFilesFileEntry, @@ -18,22 +24,24 @@ import { type ToolStatus, } from "./utils"; -/** - * Collapsed-by-default rendering for `edit_files` tool calls. - * Shows "Edited " (or "Edited N files") with a chevron; - * expanding reveals a unified diff for each file. - */ +const EDIT_FILES_AUTO_DISPLAY_STATE: AgentDisplayState = "preview"; + export const EditFilesTool: React.FC<{ files: EditFilesFileEntry[]; diffs: (FileDiffMetadata | null)[]; status: ToolStatus; isError: boolean; errorMessage?: string; -}> = ({ files, diffs, status, isError, errorMessage }) => { + codeDiffDisplayMode?: TypesGen.AgentDisplayMode; +}> = ({ files, diffs, status, isError, errorMessage, codeDiffDisplayMode }) => { const theme = useTheme(); const isDark = theme.palette.mode === "dark"; const isRunning = status === "running"; const hasDiffs = diffs.some((d) => d !== null); + const displayState = resolveAgentDisplayState( + codeDiffDisplayMode, + EDIT_FILES_AUTO_DISPLAY_STATE, + ); let label: string; if (isRunning) { @@ -54,10 +62,11 @@ export const EditFilesTool: React.FC<{ } return ( - {label} @@ -84,7 +93,11 @@ export const EditFilesTool: React.FC<{ key={files[i].path} data-testid="edit-file-diff" className="rounded-md border border-solid border-border-default text-2xs" - viewportClassName="max-h-64" + viewportClassName={ + isAgentDisplayFullyExpanded(displayState) + ? "max-h-[80vh]" + : "max-h-64" + } scrollBarClassName="w-1.5" > - + ); }; diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.stories.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.stories.tsx index 5544aa2fd6..8303f6709e 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.stories.tsx @@ -51,6 +51,10 @@ export const ExecuteSuccess: Story = { "From github.com:coder/coder\n * [new branch] feature/agent-ui -> origin/feature/agent-ui", }, }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect(canvas.getByText(/From github\.com:coder\/coder/)).toBeVisible(); + }, }; export const ExecuteAuthRequired: Story = { @@ -983,6 +987,7 @@ export const WriteFileSuccess: Story = { args: { name: "write_file", status: "completed", + codeDiffDisplayMode: "auto", args: { path: "src/utils/helpers.ts", content: @@ -992,6 +997,30 @@ export const WriteFileSuccess: Story = { play: async ({ canvasElement }) => { const canvas = within(canvasElement); expect(canvas.getByText(/Wrote helpers\.ts/)).toBeInTheDocument(); + expect(canvas.queryByTestId("write-file-diff")).not.toBeInTheDocument(); + await userEvent.click( + canvas.getByRole("button", { name: /Wrote helpers\.ts/ }), + ); + await waitFor(() => { + expect(canvas.getByTestId("write-file-diff")).toBeVisible(); + }); + }, +}; + +export const WriteFileAlwaysExpanded: Story = { + args: { + name: "write_file", + status: "completed", + codeDiffDisplayMode: "always_expanded", + args: { + path: "src/utils/helpers.ts", + content: + "export function greet(name: string): string {\n return `Hello, ${name}!`;\n}\n", + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect(canvas.getByTestId("write-file-diff")).toBeVisible(); }, }; @@ -1027,6 +1056,7 @@ export const EditFilesSingleSuccess: Story = { args: { name: "edit_files", status: "completed", + codeDiffDisplayMode: "auto", args: { files: [ { @@ -1044,6 +1074,39 @@ export const EditFilesSingleSuccess: Story = { play: async ({ canvasElement }) => { const canvas = within(canvasElement); expect(canvas.getByText(/Edited config\.ts/)).toBeInTheDocument(); + expect(canvas.getAllByTestId("edit-file-diff")).toHaveLength(1); + }, +}; + +export const EditFilesAlwaysCollapsed: Story = { + args: { + name: "edit_files", + status: "completed", + codeDiffDisplayMode: "always_collapsed", + args: { + files: [ + { + path: "src/config.ts", + edits: [ + { + search: "const timeout = 30;", + replace: "const timeout = 60;", + }, + ], + }, + ], + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect(canvas.getByText(/Edited config\.ts/)).toBeVisible(); + expect(canvas.queryAllByTestId("edit-file-diff")).toHaveLength(0); + await userEvent.click( + canvas.getByRole("button", { name: /Edited config\.ts/ }), + ); + await waitFor(() => { + expect(canvas.getAllByTestId("edit-file-diff")).toHaveLength(1); + }); }, }; diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx index 3b2d106809..1b68853506 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx @@ -1,5 +1,5 @@ import { useTheme } from "@emotion/react"; -import { FileDiff, File as FileViewer } from "@pierre/diffs/react"; +import { File as FileViewer } from "@pierre/diffs/react"; import { LoaderIcon, TriangleAlertIcon } from "lucide-react"; import { type ComponentPropsWithRef, type FC, memo } from "react"; import type * as TypesGen from "#/api/typesGenerated"; @@ -49,7 +49,6 @@ import { buildEditDiff, DIFFS_FONT_STYLE, formatResultOutput, - getDiffViewerOptions, getFileContentForViewer, getFileViewerOptions, getFileViewerOptionsNoHeader, @@ -60,7 +59,6 @@ import { parseEditFilesArgs, parseServerEditDiffText, parseServerEditResults, - stripNoNewline, type ToolStatus, toProviderLabel, } from "./utils"; @@ -94,6 +92,7 @@ interface ToolProps extends Omit, "children"> { previousResponseText?: string; /** Human-readable intent extracted from the model's tool-call args. */ modelIntent?: string; + codeDiffDisplayMode?: TypesGen.AgentDisplayMode; } // Props passed to each tool-specific renderer function. Each renderer @@ -117,6 +116,7 @@ type ToolRendererProps = { mcpServerConfigId?: string; mcpServers?: readonly TypesGen.MCPServerConfig[]; modelIntent?: string; + codeDiffDisplayMode?: TypesGen.AgentDisplayMode; }; // --------------------------------------------------------------------------- @@ -369,6 +369,7 @@ const WriteFileRenderer: FC = ({ args, result, isError, + codeDiffDisplayMode, }) => { const parsedArgs = parseArgs(args); const path = parsedArgs ? asString(parsedArgs.path).trim() : ""; @@ -382,6 +383,7 @@ const WriteFileRenderer: FC = ({ status={status} isError={isError} errorMessage={rec ? asString(rec.error || rec.message) : undefined} + codeDiffDisplayMode={codeDiffDisplayMode} /> ); }; @@ -391,6 +393,7 @@ const EditFilesRenderer: FC = ({ args, result, isError, + codeDiffDisplayMode, }) => { const rec = asRecord(result); const editFiles = parseEditFilesArgs(args); @@ -413,6 +416,7 @@ const EditFilesRenderer: FC = ({ status={status} isError={isError} errorMessage={rec ? asString(rec.error || rec.message) : undefined} + codeDiffDisplayMode={codeDiffDisplayMode} /> ); }; @@ -814,8 +818,6 @@ const ComputerRenderer: FC = ({ ); }; -// Generic fallback renderer — only path that needs theme, diff -// viewers, and file content helpers. const GenericToolRenderer: FC = ({ name, status, @@ -830,7 +832,6 @@ const GenericToolRenderer: FC = ({ const isDark = theme.palette.mode === "dark"; const resultOutput = formatResultOutput(result); const fileContent = getFileContentForViewer(name, args, result); - const writeFileDiff = getWriteFileDiff(name, args); const fileViewerOpts = getFileViewerOptions(isDark); const fileContentOptions = fileContent ? { @@ -845,64 +846,49 @@ const GenericToolRenderer: FC = ({ ? mcpServers?.find((s) => s.id === mcpServerConfigId) : undefined; - const hasContent = Boolean(writeFileDiff || fileContent || resultOutput); + const hasContent = Boolean(fileContent || resultOutput); const isRunning = status === "running"; const rec = asRecord(result); const errorMessage = rec ? asString(rec.error || rec.message) : ""; - return ( - - - {modelIntent ? ( - - {modelIntent.charAt(0).toUpperCase() + modelIntent.slice(1)} - - ) : ( - - )} - {isError && ( - - - - - - {errorMessage || "Tool call failed"} - - - )} - {isRunning && ( - - )} - - } - > - {writeFileDiff ? ( - - - - ) : fileContent ? ( + const toolHeader = ( + <> + + {modelIntent ? ( + + {modelIntent.charAt(0).toUpperCase() + modelIntent.slice(1)} + + ) : ( + + )} + {isError && ( + + + + + {errorMessage || "Tool call failed"} + + )} + {isRunning && ( + + )} + + ); + + const toolContent = ( + <> + {fileContent ? ( = ({ ) )} + + ); + + return ( + + {toolContent} ); }; @@ -1030,6 +1022,7 @@ export const Tool = memo( isLatestAskUserQuestion, previousResponseText, modelIntent, + codeDiffDisplayMode, ref, ...props }: ToolProps) => { @@ -1076,6 +1069,7 @@ export const Tool = memo( isLatestAskUserQuestion={isLatestAskUserQuestion} previousResponseText={previousResponseText} modelIntent={modelIntent} + codeDiffDisplayMode={codeDiffDisplayMode} /> ); diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/ToolCollapsible.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/ToolCollapsible.tsx index 2ff075a1b7..5cb279d820 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/ToolCollapsible.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/ToolCollapsible.tsx @@ -1,7 +1,13 @@ import { ChevronDownIcon } from "lucide-react"; import type { FC, ReactNode } from "react"; import { useState } from "react"; +import type { AgentDisplayMode } from "#/api/typesGenerated"; import { cn } from "#/utils/cn"; +import { + type AgentDisplayState, + isAgentDisplayOpen, + resolveAgentDisplayState, +} from "./displayMode"; type ToolCollapsibleHeader = ReactNode | ((expanded: boolean) => ReactNode); @@ -14,6 +20,25 @@ interface ToolCollapsibleProps { headerClassName?: string; } +interface AgentDisplayModeToolCollapsibleProps + extends Omit { + displayMode: AgentDisplayMode | undefined; + autoDisplayState: AgentDisplayState; +} + +export const AgentDisplayModeToolCollapsible: FC< + AgentDisplayModeToolCollapsibleProps +> = ({ displayMode, autoDisplayState, ...props }) => { + const displayState = resolveAgentDisplayState(displayMode, autoDisplayState); + return ( + + ); +}; + export const ToolCollapsible: FC = ({ children, header, diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/WriteFileTool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/WriteFileTool.tsx index fa79904c70..c98bea1e22 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/WriteFileTool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/WriteFileTool.tsx @@ -3,13 +3,19 @@ import type { FileDiffMetadata } from "@pierre/diffs"; import { FileDiff } from "@pierre/diffs/react"; import { LoaderIcon, TriangleAlertIcon } from "lucide-react"; import type React from "react"; +import type * as TypesGen from "#/api/typesGenerated"; import { ScrollArea } from "#/components/ScrollArea/ScrollArea"; import { Tooltip, TooltipContent, TooltipTrigger, } from "#/components/Tooltip/Tooltip"; -import { ToolCollapsible } from "./ToolCollapsible"; +import { + type AgentDisplayState, + isAgentDisplayFullyExpanded, + resolveAgentDisplayState, +} from "./displayMode"; +import { AgentDisplayModeToolCollapsible } from "./ToolCollapsible"; import { DIFFS_FONT_STYLE, getDiffViewerOptions, @@ -17,29 +23,34 @@ import { type ToolStatus, } from "./utils"; -/** - * Collapsed-by-default rendering for `write_file` tool calls. Shows - * "Wrote " with a chevron; expanding reveals the unified diff. - */ +const WRITE_FILE_AUTO_DISPLAY_STATE: AgentDisplayState = "collapsed"; + export const WriteFileTool: React.FC<{ path: string; diff: FileDiffMetadata | null; status: ToolStatus; isError: boolean; errorMessage?: string; -}> = ({ path, diff, status, isError, errorMessage }) => { + codeDiffDisplayMode?: TypesGen.AgentDisplayMode; +}> = ({ path, diff, status, isError, errorMessage, codeDiffDisplayMode }) => { const theme = useTheme(); const isDark = theme.palette.mode === "dark"; const hasDiff = diff !== null; const isRunning = status === "running"; + const displayState = resolveAgentDisplayState( + codeDiffDisplayMode, + WRITE_FILE_AUTO_DISPLAY_STATE, + ); const filename = path.split("/").pop() || path; const label = isRunning ? `Writing ${filename}…` : `Wrote ${filename}`; return ( - {label} @@ -61,8 +72,13 @@ export const WriteFileTool: React.FC<{ > {hasDiff && ( )} - + ); }; diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/displayMode.test.ts b/site/src/pages/AgentsPage/components/ChatElements/tools/displayMode.test.ts new file mode 100644 index 0000000000..9b66e3f6ee --- /dev/null +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/displayMode.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from "vitest"; + +import { + isAgentDisplayFullyExpanded, + isAgentDisplayOpen, + resolveAgentDisplayState, +} from "./displayMode"; + +describe("resolveAgentDisplayState", () => { + it("resolves auto and explicit display modes", () => { + expect(resolveAgentDisplayState(undefined, "preview")).toBe("preview"); + expect(resolveAgentDisplayState("auto", "collapsed")).toBe("collapsed"); + expect(resolveAgentDisplayState("auto", "expanded")).toBe("expanded"); + expect(resolveAgentDisplayState("always_expanded", "collapsed")).toBe( + "expanded", + ); + expect(resolveAgentDisplayState("always_collapsed", "expanded")).toBe( + "collapsed", + ); + }); +}); + +describe("isAgentDisplayOpen", () => { + it("returns whether a display state shows content", () => { + expect(isAgentDisplayOpen("collapsed")).toBe(false); + expect(isAgentDisplayOpen("preview")).toBe(true); + expect(isAgentDisplayOpen("expanded")).toBe(true); + }); +}); + +describe("isAgentDisplayFullyExpanded", () => { + it("returns whether a display state uses a fully expanded view", () => { + expect(isAgentDisplayFullyExpanded("expanded")).toBe(true); + expect(isAgentDisplayFullyExpanded("preview")).toBe(false); + expect(isAgentDisplayFullyExpanded("collapsed")).toBe(false); + }); +}); diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/displayMode.ts b/site/src/pages/AgentsPage/components/ChatElements/tools/displayMode.ts new file mode 100644 index 0000000000..78d7ce6bd3 --- /dev/null +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/displayMode.ts @@ -0,0 +1,32 @@ +import type { AgentDisplayMode } from "#/api/typesGenerated"; + +export type AgentDisplayState = "collapsed" | "preview" | "expanded"; + +export const resolveAgentDisplayState = ( + mode: AgentDisplayMode | undefined, + autoState: AgentDisplayState, +): AgentDisplayState => { + switch (mode) { + case undefined: + case "auto": + return autoState; + case "always_expanded": + return "expanded"; + case "always_collapsed": + return "collapsed"; + default: { + const _exhaustive: never = mode; + return _exhaustive; + } + } +}; + +export const isAgentDisplayOpen = (state: AgentDisplayState): boolean => { + return state !== "collapsed"; +}; + +export const isAgentDisplayFullyExpanded = ( + state: AgentDisplayState, +): boolean => { + return state === "expanded"; +}; diff --git a/site/src/pages/AgentsPage/components/DisplayModeSettings.tsx b/site/src/pages/AgentsPage/components/DisplayModeSettings.tsx new file mode 100644 index 0000000000..2e0b861ff5 --- /dev/null +++ b/site/src/pages/AgentsPage/components/DisplayModeSettings.tsx @@ -0,0 +1,133 @@ +import type { FC } from "react"; +import { useMutation, useQuery, useQueryClient } from "react-query"; +import { + preferenceSettings, + updatePreferenceSettings, +} from "#/api/queries/users"; +import type { + UpdateUserPreferenceSettingsRequest, + UserPreferenceSettings, +} from "#/api/typesGenerated"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "#/components/Select/Select"; + +type DisplayModeOption = { value: T; label: string }; + +type ThinkingDisplayMode = UserPreferenceSettings["thinking_display_mode"]; +type AgentDisplayMode = UserPreferenceSettings["code_diff_display_mode"]; + +const thinkingDisplayOptions: DisplayModeOption[] = [ + { value: "auto", label: "Auto" }, + { value: "preview", label: "Preview" }, + { value: "always_expanded", label: "Always Expanded" }, + { value: "always_collapsed", label: "Always Collapsed" }, +]; + +const agentDisplayOptions: DisplayModeOption[] = [ + { value: "auto", label: "Auto" }, + { value: "always_expanded", label: "Always Expanded" }, + { value: "always_collapsed", label: "Always Collapsed" }, +]; + +type DisplayModeSettingsProps = { + title: string; + description: string; + ariaLabel: string; + errorMessage: string; + defaultValue: T; + options: DisplayModeOption[]; + getMode: (settings: UserPreferenceSettings) => T; + updateSettings: (value: T) => UpdateUserPreferenceSettingsRequest; +}; + +const DisplayModeSettings = ({ + title, + description, + ariaLabel, + errorMessage, + defaultValue, + options, + getMode, + updateSettings, +}: DisplayModeSettingsProps) => { + const queryClient = useQueryClient(); + const query = useQuery(preferenceSettings()); + const mutation = useMutation(updatePreferenceSettings(queryClient)); + + const mode = query.data ? getMode(query.data) : defaultValue; + + return ( +
+

+ {title} +

+
+

+ {description} +

+ +
+ {mutation.isError && ( +

{errorMessage}

+ )} +
+ ); +}; + +export const ThinkingDisplaySettings: FC = () => { + return ( + settings.thinking_display_mode} + updateSettings={(value) => ({ + thinking_display_mode: value, + })} + /> + ); +}; + +export const CodeDiffDisplaySettings: FC = () => { + return ( + settings.code_diff_display_mode} + updateSettings={(value) => ({ + code_diff_display_mode: value, + })} + /> + ); +}; diff --git a/site/src/pages/AgentsPage/components/ThinkingDisplaySettings.tsx b/site/src/pages/AgentsPage/components/ThinkingDisplaySettings.tsx deleted file mode 100644 index 07422f0627..0000000000 --- a/site/src/pages/AgentsPage/components/ThinkingDisplaySettings.tsx +++ /dev/null @@ -1,75 +0,0 @@ -import type { FC } from "react"; -import { useMutation, useQuery, useQueryClient } from "react-query"; -import { - preferenceSettings, - updatePreferenceSettings, -} from "#/api/queries/users"; -import type { ThinkingDisplayMode } from "#/api/typesGenerated"; -import { - Select, - SelectContent, - SelectItem, - SelectTrigger, - SelectValue, -} from "#/components/Select/Select"; - -const options: { value: ThinkingDisplayMode; label: string }[] = [ - { value: "auto", label: "Auto" }, - { value: "preview", label: "Preview" }, - { value: "always_expanded", label: "Always Expanded" }, - { value: "always_collapsed", label: "Always Collapsed" }, -]; - -export const ThinkingDisplaySettings: FC = () => { - const queryClient = useQueryClient(); - const query = useQuery(preferenceSettings()); - const mutation = useMutation(updatePreferenceSettings(queryClient)); - - const mode: ThinkingDisplayMode = query.data?.thinking_display_mode || "auto"; - - return ( -
-

- Thinking Display -

-
-

- How thinking blocks should be displayed by default. 'Auto' fully - expands during streaming, then auto-collapses when done. 'Preview' - auto-expands with a height constraint during streaming. 'Always - Expanded' shows full content. 'Always Collapsed' keeps them collapsed. -

- -
- {mutation.isError && ( -

- Failed to save your thinking display preference. -

- )} -
- ); -}; diff --git a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.stories.tsx b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.stories.tsx index b3098464d6..1c35286761 100644 --- a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.stories.tsx +++ b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.stories.tsx @@ -215,6 +215,7 @@ export const EnablingTaskNotificationClearsAlertDismissal: Story = { data: { task_notification_alert_dismissed: true, thinking_display_mode: "auto" as const, + code_diff_display_mode: "auto" as const, }, }, ], @@ -238,6 +239,7 @@ export const EnablingTaskNotificationClearsAlertDismissal: Story = { ).mockResolvedValue({ task_notification_alert_dismissed: false, thinking_display_mode: "auto", + code_diff_display_mode: "auto", }); await step("Enable Task Idle notification", async () => {