ronantakizawa/codereview-bench huggingface dataset:
Repository: streamlit/streamlit
File: frontend/app/src/App.test.tsx (TypeScript)

Code before:
            expect.objectContaining({
              name: CUSTOM_THEME_LIGHT_NAME,
              displayName: "Light",
            })
          )

          window.localStorage.removeItem(LocalStore.ACTIVE_THEME)
        })
      })
    })
  })

  describe("embed_options with custom themes", () => {
    let windowSpy: MockInstance

    beforeEach(() => {
      window.localStorage.clear()
    })

    afterEach(() => {
      windowSpy?.mockRestore()
    })

    it("respects embed_options=light_theme over cached dark preference", () => {
      // Set cached theme to dark
      const cachedTheme: CachedTheme = {
        name: CUSTOM_THEME_DARK_NAME,
      }
      window.localStorage.setItem(
        LocalStore.ACTIVE_THEME,
        JSON.stringify(cachedTheme)
      )

      // Mock query params with light_theme
      windowSpy = mockWindow(
        windowLocationSearch("?embed=true&embed_options=light_theme")
      )

      const props = getProps()
      renderApp(props)

      // Send custom theme config with light and dark sections
      const customTheme = new CustomThemeConfig({
        primaryColor: "blue",
        light: {
          backgroundColor: "white",
        },
        dark: {
          backgroundColor: "black",
        },
      })

Review comment (performance):
**Nit**: `CachedTheme` will need to be exported from `@streamlit/lib` by adding to `frontend/lib/src/index.ts`:
```javascript
export type { CachedTheme, EmotionTheme, PresetThemeName, ThemeConfig } from "./theme"
```

Code after:
          // Should set theme to update with full server config while preserving Light selection (FIX FOR #13280)
          expect(props.theme.setTheme).toHaveBeenCalledWith(
            expect.objectContaining({
              name: CUSTOM_THEME_LIGHT_NAME,
              displayName: "Light",
            })
          )

          window.localStorage.removeItem(LocalStore.ACTIVE_THEME)
        })
      })
    })
  })

  describe("embed_options with custom themes", () => {
    let prevWindowLocation: Location

    beforeEach(() => {
      window.localStorage.clear()
      prevWindowLocation = window.location
    })

    afterEach(() => {
      Object.defineProperty(window, "location", {
        value: prevWindowLocation,
        writable: true,
        configurable: true,
      })
    })

    const setLocationSearch = (search: string): void => {
      Object.defineProperty(window, "location", {
        value: {
          ...prevWindowLocation,
          search,
          href: `http://localhost/${search}`,
        },
        writable: true,
        configurable: true,
      })
    }

    const customTheme = new CustomThemeConfig({
      primaryColor: "blue",
      light: {
        backgroundColor: "white",
      },
      dark: {
        backgroundColor: "black",
      },
    })
Repository: streamlit/streamlit
File: frontend/app/src/App.test.tsx (TypeScript)

Code before:

  describe("embed_options with custom themes", () => {
    let windowSpy: MockInstance

    beforeEach(() => {
      window.localStorage.clear()
    })

    afterEach(() => {
      windowSpy?.mockRestore()
    })

    it("respects embed_options=light_theme over cached dark preference", () => {
      // Set cached theme to dark
      const cachedTheme: CachedTheme = {
        name: CUSTOM_THEME_DARK_NAME,
      }
      window.localStorage.setItem(
        LocalStore.ACTIVE_THEME,
        JSON.stringify(cachedTheme)
      )

      // Mock query params with light_theme
      windowSpy = mockWindow(
        windowLocationSearch("?embed=true&embed_options=light_theme")
      )

      const props = getProps()
      renderApp(props)

      // Send custom theme config with light and dark sections
      const customTheme = new CustomThemeConfig({
        primaryColor: "blue",
        light: {
          backgroundColor: "white",
        },
        dark: {
          backgroundColor: "black",
        },
      })

      sendForwardMessage("newSession", {
        ...NEW_SESSION_JSON,
        customTheme,
      })

      // Should apply Custom Theme Light (from embed_options), not Dark (from cache)
      expect(props.theme.setTheme).toHaveBeenCalled()
      const appliedTheme = vi.mocked(props.theme.setTheme).mock.calls[0][0]
      expect(appliedTheme.name).toBe(CUSTOM_THEME_LIGHT_NAME)
      expect(appliedTheme.emotion.colors.bgColor).toBe("white")

Review comment (refactor):
The call here (and in others below) could then use the helper:
```javascript
// Mock query params with light_theme
setLocationSearch("?embed=true&embed_options=light_theme")
```

Code after:
    })
  })

  describe("embed_options with custom themes", () => {
    let prevWindowLocation: Location

    beforeEach(() => {
      window.localStorage.clear()
      prevWindowLocation = window.location
    })

    afterEach(() => {
      Object.defineProperty(window, "location", {
        value: prevWindowLocation,
        writable: true,
        configurable: true,
      })
    })

    const setLocationSearch = (search: string): void => {
      Object.defineProperty(window, "location", {
        value: {
          ...prevWindowLocation,
          search,
          href: `http://localhost/${search}`,
        },
        writable: true,
        configurable: true,
      })
    }

    const customTheme = new CustomThemeConfig({
      primaryColor: "blue",
      light: {
        backgroundColor: "white",
      },
      dark: {
        backgroundColor: "black",
      },
    })
    const cachedLightTheme: CachedTheme = {
      name: CUSTOM_THEME_LIGHT_NAME,
      themeInput: customTheme,
    }
    const cachedDarkTheme: CachedTheme = {
      name: CUSTOM_THEME_DARK_NAME,
      themeInput: customTheme,
    }

    it("respects embed_options=light_theme over cached dark preference", () => {
      // Set cached theme to dark
Repository: streamlit/streamlit
File: frontend/app/src/App.test.tsx (TypeScript)

Code before:

    afterEach(() => {
      Object.defineProperty(window, "location", {
        value: prevWindowLocation,
        writable: true,
        configurable: true,
      })
    })

    const setLocationSearch = (search: string): void => {
      Object.defineProperty(window, "location", {
        value: {
          ...prevWindowLocation,
          search,
          href: `http://localhost/${search}`,
        },
        writable: true,
        configurable: true,
      })
    }

    it("respects embed_options=light_theme over cached dark preference", () => {
      // Set cached theme to dark
      const cachedTheme: CachedTheme = {
        name: CUSTOM_THEME_DARK_NAME,
      }
      window.localStorage.setItem(
        LocalStore.ACTIVE_THEME,
        JSON.stringify(cachedTheme)
      )

      // Mock query params with light_theme
      setLocationSearch("?embed=true&embed_options=light_theme")

      const props = getProps()
      renderApp(props)

      // Send custom theme config with light and dark sections
      const customTheme = new CustomThemeConfig({
        primaryColor: "blue",
        light: {
          backgroundColor: "white",
        },
        dark: {
          backgroundColor: "black",
        },
      })

      sendForwardMessage("newSession", {
        ...NEW_SESSION_JSON,
        customTheme,

Review comment (performance):
**Nit**: `cachedTheme` needs to have a `name` and `themeInput`
```javascript
// Set cached theme to dark (with required themeInput for custom themes)
const cachedTheme: CachedTheme = {
  name: CUSTOM_THEME_DARK_NAME,
  themeInput: { primaryColor: "blue" },
}
```


Code after:
    })

    afterEach(() => {
      Object.defineProperty(window, "location", {
        value: prevWindowLocation,
        writable: true,
        configurable: true,
      })
    })

    const setLocationSearch = (search: string): void => {
      Object.defineProperty(window, "location", {
        value: {
          ...prevWindowLocation,
          search,
          href: `http://localhost/${search}`,
        },
        writable: true,
        configurable: true,
      })
    }

    const customTheme = new CustomThemeConfig({
      primaryColor: "blue",
      light: {
        backgroundColor: "white",
      },
      dark: {
        backgroundColor: "black",
      },
    })
    const cachedLightTheme: CachedTheme = {
      name: CUSTOM_THEME_LIGHT_NAME,
      themeInput: customTheme,
    }
    const cachedDarkTheme: CachedTheme = {
      name: CUSTOM_THEME_DARK_NAME,
      themeInput: customTheme,
    }

    it("respects embed_options=light_theme over cached dark preference", () => {
      // Set cached theme to dark
      window.localStorage.setItem(
        LocalStore.ACTIVE_THEME,
        JSON.stringify(cachedDarkTheme)
      )

      // Mock query params with light_theme
      setLocationSearch("?embed=true&embed_options=light_theme")

      const props = getProps()
Repository: streamlit/streamlit
File: frontend/lib/src/components/widgets/ChatInput/styled-components.ts (TypeScript)

Code before:
      return theme.colors.fadedText60
    }

    return {
      border: "none",
      backgroundColor: theme.colors.transparent,
      borderRadius: theme.radii.default,
      display: "inline-flex",
      alignItems: "center",
      justifyContent: "center",
      lineHeight: theme.lineHeights.none,
      margin: theme.spacing.none,
      padding: theme.spacing.none,
      color: getSendIconColor(),
      pointerEvents: "auto",
      "&:focus": {
        outline: "none",
      },
      ":focus": {
        outline: "none",
      },
      "&:focus-visible": {
        boxShadow: getPrimaryFocusBoxShadow(theme),
      },
      "&:hover": {
        color: hasError ? theme.colors.red70 : theme.colors.bodyText,
      },
      "&:active": {
        color: theme.colors.primary,
      },
      "&:disabled, &:disabled:hover, &:disabled:active": {
        backgroundColor: theme.colors.transparent,
        borderColor: theme.colors.transparent,
        color: theme.colors.fadedText40,
        cursor: "not-allowed",
      },
      "& svg": {
        width: theme.iconSizes.lg,
        height: theme.iconSizes.lg,
      },
    }
  }
)

interface StyledWaveformContainerProps {
  isRecording: boolean
}

export const StyledWaveformContainer =
  styled.div<StyledWaveformContainerProps>(({ isRecording }) => ({
    display: isRecording ? "flex" : "none",

Review comment (suggestion):
**question:** It might make sense to use the configurable red color here `theme.colors.redColor`  (also red70 in light mode and red80 in dark mode). 

Code after:
      return theme.colors.fadedText60
    }

    return {
      border: "none",
      backgroundColor: theme.colors.transparent,
      borderRadius: theme.radii.default,
      display: "inline-flex",
      alignItems: "center",
      justifyContent: "center",
      lineHeight: theme.lineHeights.none,
      margin: theme.spacing.none,
      padding: theme.spacing.none,
      color: getSendIconColor(),
      pointerEvents: "auto",
      "&:focus": {
        outline: "none",
      },
      ":focus": {
        outline: "none",
      },
      "&:focus-visible": {
        boxShadow: getPrimaryFocusBoxShadow(theme),
      },
      "&:hover": {
        color: hasError ? theme.colors.redColor : theme.colors.bodyText,
      },
      "&:active": {
        color: theme.colors.primary,
      },
      "&:disabled, &:disabled:hover, &:disabled:active": {
        backgroundColor: theme.colors.transparent,
        borderColor: theme.colors.transparent,
        color: theme.colors.fadedText40,
        cursor: "not-allowed",
      },
      "& svg": {
        width: theme.iconSizes.lg,
        height: theme.iconSizes.lg,
      },
    }
  }
)

interface StyledWaveformContainerProps {
  isRecording: boolean
}

export const StyledWaveformContainer =
  styled.div<StyledWaveformContainerProps>(({ isRecording }) => ({
    display: isRecording ? "flex" : "none",
Repository: streamlit/streamlit
File: e2e_playwright/st_expander_test.py (Python)

Code before:
    expect(app.get_by_text("Callback last state: None")).to_be_visible()

    # Expand the callback expander
    cb_expander = get_expander(app, "Callback expander")
    cb_expander.locator("summary").click()
    wait_for_app_run(app)

    # Callback should have fired, state should be True (expanded)
    expect(app.get_by_text("Callback count: 1")).to_be_visible()
    expect(app.get_by_text("Callback last state: True")).to_be_visible()

    # Collapse the callback expander
    cb_expander = get_expander(app, "Callback expander")
    cb_expander.locator("summary").click()
    wait_for_app_run(app)

    # Callback should have fired again, state should be False (collapsed)
    expect(app.get_by_text("Callback count: 2")).to_be_visible()
    expect(app.get_by_text("Callback last state: False")).to_be_visible()


def test_expander_callback_with_args_kwargs(app: Page):
    """Test that a callback with args and kwargs receives them correctly."""
    # Initially no result
    expect(app.get_by_text("Callback args result:")).to_be_visible()

    # Expand the callback args expander
    cb_args_expander = get_expander(app, "Callback args expander")
    cb_args_expander.locator("summary").click()
    wait_for_app_run(app)

    # Callback should have received args and kwargs
    expect(app.get_by_text("Callback args result: hello-toggled-world")).to_be_visible()


Review comment (suggestion):
**suggestion:** Consider doing a direct string match rather than a substring match here. I believe as written this will pass for both `Callback args result:` and `Callback args result: hello-toggled-world`.

Code after:
    expect(app.get_by_text("Callback last state: None")).to_be_visible()

    # Expand the callback expander
    cb_expander = get_expander(app, "Callback expander")
    cb_expander.locator("summary").click()
    wait_for_app_run(app)

    # Callback should have fired, state should be True (expanded)
    expect(app.get_by_text("Callback count: 1")).to_be_visible()
    expect(app.get_by_text("Callback last state: True")).to_be_visible()

    # Collapse the callback expander
    cb_expander = get_expander(app, "Callback expander")
    cb_expander.locator("summary").click()
    wait_for_app_run(app)

    # Callback should have fired again, state should be False (collapsed)
    expect(app.get_by_text("Callback count: 2")).to_be_visible()
    expect(app.get_by_text("Callback last state: False")).to_be_visible()


def test_expander_callback_with_args_kwargs(app: Page):
    """Test that a callback with args and kwargs receives them correctly."""
    # Initially no result
    expect(app.get_by_text("Callback args result:", exact=True)).to_be_visible()

    # Expand the callback args expander
    cb_args_expander = get_expander(app, "Callback args expander")
    cb_args_expander.locator("summary").click()
    wait_for_app_run(app)

    # Callback should have received args and kwargs
    expect(app.get_by_text("Callback args result: hello-toggled-world")).to_be_visible()
Repository: sunflower-farmers/sunflower-farmers
File: src/lib/flags.ts (TypeScript)

Code before:
  HOARDING_CHECK: defaultFeatureFlag,
  STREAMER_HAT: (game) =>
    (game.wardrobe["Streamer Hat"] ?? 0) > 0 || testnetFeatureFlag(),

  // Temporary Feature Flags
  FACE_RECOGNITION_TEST: defaultFeatureFlag,
  LEDGER: testnetLocalStorageFeatureFlag("ledger"),

  EASTER: () => false,

  STREAM_STAGE_ACCESS: adminFeatureFlag,

  WITHDRAWAL_THRESHOLD: timePeriodFeatureFlag({
    start: new Date("2025-05-08T00:00:00Z"),
    end: new Date("2025-06-20T00:00:00.000Z"),
  }),

  MODERATOR: (game) =>
    !!((game.wardrobe.Halo ?? 0) > 0) && !!game.inventory["Beta Pass"]?.gt(0),

  BLESSING: () => true,

  CRAFTING: betaTimeBasedFeatureFlag(SEASONS["Better Together"].startDate),

  TRASH_BIN_CARRY_OVER_LIMIT: timeBasedFeatureFlag(
    new Date("2025-08-08T00:00:00Z"),
  ),

  CHEERS_V2: defaultFeatureFlag,
} satisfies Record<string, FeatureFlag>;

export type FeatureName = keyof typeof FEATURE_FLAGS;

export const hasFeatureAccess = (game: GameState, featureName: FeatureName) => {
  return FEATURE_FLAGS[featureName](game);
};


Review comment (suggestion):
Maybe you can make it an additional day so we can have more testing
```suggestion
    new Date("2025-08-09T00:00:00Z"),
```

Code after:
  HOARDING_CHECK: defaultFeatureFlag,
  STREAMER_HAT: (game) =>
    (game.wardrobe["Streamer Hat"] ?? 0) > 0 || testnetFeatureFlag(),

  // Temporary Feature Flags
  FACE_RECOGNITION_TEST: defaultFeatureFlag,
  LEDGER: testnetLocalStorageFeatureFlag("ledger"),

  EASTER: () => false,

  STREAM_STAGE_ACCESS: adminFeatureFlag,

  WITHDRAWAL_THRESHOLD: timePeriodFeatureFlag({
    start: new Date("2025-05-08T00:00:00Z"),
    end: new Date("2025-06-20T00:00:00.000Z"),
  }),

  MODERATOR: (game) =>
    !!((game.wardrobe.Halo ?? 0) > 0) && !!game.inventory["Beta Pass"]?.gt(0),

  BLESSING: () => true,

  CRAFTING: betaTimeBasedFeatureFlag(SEASONS["Better Together"].startDate),

  TRASH_BIN_CARRY_OVER_LIMIT: betaTimeBasedFeatureFlag(
    new Date("2025-08-09T00:00:00Z"),
  ),

  CHEERS_V2: defaultFeatureFlag,
} satisfies Record<string, FeatureFlag>;

export type FeatureName = keyof typeof FEATURE_FLAGS;

export const hasFeatureAccess = (game: GameState, featureName: FeatureName) => {
  return FEATURE_FLAGS[featureName](game);
};
Repository: sunflower-farmers/sunflower-farmers
File: src/features/marketplace/lib/getCollectionName.ts (TypeScript)

Code before:
import { KNOWN_IDS } from "features/game/types";
import { BumpkinItem, ITEM_IDS } from "features/game/types/bumpkin";
import { InventoryItemName } from "features/game/types/game";
import {
  MarketplaceTradeableName,
  CollectionName,
} from "features/game/types/marketplace";

export function getCollectionName(
  itemName: MarketplaceTradeableName,
): Exclude<CollectionName, "resources"> {
  if ((itemName as InventoryItemName) in KNOWN_IDS) {
    return "collectibles";
  }

  if ((itemName as BumpkinItem) in ITEM_IDS) {
    return "wearables";
  }

  if (itemName.startsWith("Bud")) {
    return "buds";
  }

  throw new Error("Unknown collection");
}


Review comment (suggestion):
I think we can do away with type casting here as it's not really causing any errors with Typescript.

```suggestion
  if (itemName in KNOWN_IDS) {
    return "collectibles";
  }

  if (itemName in ITEM_IDS) {
    return "wearables";
  }
```

Code after:
import { KNOWN_IDS } from "features/game/types";
import { ITEM_IDS } from "features/game/types/bumpkin";
import {
  MarketplaceTradeableName,
  CollectionName,
} from "features/game/types/marketplace";

export function getCollectionName(
  itemName: MarketplaceTradeableName,
): Exclude<CollectionName, "resources"> {
  if (itemName in KNOWN_IDS) {
    return "collectibles";
  }

  if (itemName in ITEM_IDS) {
    return "wearables";
  }

  if (itemName.startsWith("Bud")) {
    return "buds";
  }

  throw new Error("Unknown collection");
}
Repository: sunflower-farmers/sunflower-farmers
File: src/App.tsx (TypeScript)

Code before:
/* eslint-disable no-console */
import React, { useEffect } from "react";

import { initialise } from "lib/utils/init";

import "./styles.css";
import * as Auth from "features/auth/lib/Provider";
import ErrorBoundary from "features/auth/components/ErrorBoundary";
import { Navigation } from "./Navigation";
import "./lib/i18n";
import { WalletProvider } from "features/wallet/WalletProvider";

// Initialise Global Settings
initialise();

// Android + Metamask browser has issues loading assets when the Service Worker is running
// The cause is unknown hence we unregister the service worker for these users to prevent
// rendering issues
const NoServiceWorker = () => {
  useEffect(() => {
    (async () => {
      if ("serviceWorker" in navigator) {
        const registrations = await navigator.serviceWorker.getRegistrations();
        const unregisterPromises = registrations.map((registration) =>
          registration.unregister(),
        );

        const allCaches = await caches.keys();
        const cacheDeletionPromises = allCaches.map((cache) =>
          caches.delete(cache),
        );

        await Promise.all([...unregisterPromises, ...cacheDeletionPromises]);
      }
    })();
  }, []);

Review comment (suggestion):
```suggestion
import { WalletProvider } from "features/wallet/WalletProvider";
import { useServiceWorkerUpdate } from "lib/utils/hooks/useServiceWorkerUpdate";
```

Code after:
/* eslint-disable no-console */
import React, { useEffect } from "react";

import { initialise } from "lib/utils/init";

import "./styles.css";
import * as Auth from "features/auth/lib/Provider";
import ErrorBoundary from "features/auth/components/ErrorBoundary";
import { Navigation } from "./Navigation";
import "./lib/i18n";
import { WalletProvider } from "features/wallet/WalletProvider";
import { useServiceWorkerUpdate } from "lib/utils/hooks/useServiceWorkerUpdate";

// Initialise Global Settings
initialise();

// Android + Metamask browser has issues loading assets when the Service Worker is running
// The cause is unknown hence we unregister the service worker for these users to prevent
// rendering issues
const NoServiceWorker = () => {
  useEffect(() => {
    (async () => {
      if ("serviceWorker" in navigator) {
        const registrations = await navigator.serviceWorker.getRegistrations();
        const unregisterPromises = registrations.map((registration) =>
          registration.unregister(),
        );

        const allCaches = await caches.keys();
        const cacheDeletionPromises = allCaches.map((cache) =>
          caches.delete(cache),
        );

        await Promise.all([...unregisterPromises, ...cacheDeletionPromises]);
      }
    })();
Repository: sunflower-farmers/sunflower-farmers
File: src/features/island/pets/lib/petShared.ts (TypeScript)

Code before:
    asleep: pipAsleep,
  },
  Skipper: {
    happy: ITEM_DETAILS.Skipper.image,
    asleep: skipperAsleep,
  },
  Ramsey: {
    happy: ITEM_DETAILS.Ramsey.image,
    asleep: ITEM_DETAILS.Ramsey.image,
  },
};

export const _petData = (name: PetName) => (state: MachineState) =>
  state.context.state.pets?.common?.[name];

export const isPetNFT = (petId: PetName | number): petId is number => {
  return typeof petId === "number";
};

export const getPetImage = (
  state: "asleep" | "happy",
  id: number | PetName,
) => {
  if (isPetNFT(id)) {
    const isRevealed = isPetNFTRevealed(id, Date.now());
    if (!isRevealed) {
      return petNFTEgg;
    }

    // TODO: Fetch Pet NFT image
    return ITEM_DETAILS["Ramsey"].image as string;
  }

  return PET_STATE_IMAGES[id][state];
};


Review comment (suggestion):
I see this a bit where there is a creation of a variable when we don't really need one. This function name is descriptive enough and its only being used in one place so I think it can just be used inline.

```suggestion
    if (!isPetNFTRevealed(id, Date.now())) {
```

Code after:
    asleep: pipAsleep,
  },
  Skipper: {
    happy: ITEM_DETAILS.Skipper.image,
    asleep: skipperAsleep,
  },
  Ramsey: {
    happy: ITEM_DETAILS.Ramsey.image,
    asleep: ITEM_DETAILS.Ramsey.image,
  },
};

export const _petData = (name: PetName) => (state: MachineState) =>
  state.context.state.pets?.common?.[name];

export const isPetNFT = (petId: PetName | number): petId is number => {
  return typeof petId === "number";
};

export const getPetImage = (
  state: "asleep" | "happy",
  id: number | PetName,
) => {
  if (isPetNFT(id)) {
    if (!isPetNFTRevealed(id, Date.now())) {
      return petNFTEgg;
    }

    // TODO: Fetch Pet NFT image
    return ITEM_DETAILS["Ramsey"].image as string;
  }

  return PET_STATE_IMAGES[id][state];
};