Skip to content

Conversation

urmauur
Copy link
Member

@urmauur urmauur commented Jul 31, 2025

Describe Your Changes

This pull request removes the auto-refresh functionality for models of non-predefined providers in the ProviderDetail component. The change simplifies the code by delegating the responsibility for handling settings changes to a global event handler.

Key change:

  • Code simplification in ProviderDetail component:
    • Removed the useEffect hook that automatically refreshed models every 10 seconds for non-predefined providers. This functionality is now managed globally via the GlobalEventHandler, ensuring consistent handling of settings changes across all screens. ([web-app/src/routes/settings/providers/$providerName.tsxL136-L151](https://github.com/menloresearch/jan/pull/6002/files#diff-bf67bc43ea4ed453c69e14c27bd2fe2006545cb6cedc1fbad166857e073303b5L136-L151))

Fixes Issues

  • Closes #
  • Closes #

Self Checklist

  • Added relevant comments, esp in complex areas
  • Updated docs (for bug fixes / features)
  • Created issues for follow-up changes or refactoring needed

Important

Remove auto-refresh for non-predefined providers in ProviderDetail, now managed globally via GlobalEventHandler.

  • Behavior:
    • Removed useEffect hook in ProviderDetail that auto-refreshed models every 10 seconds for non-predefined providers.
    • Auto-refresh is now managed globally via GlobalEventHandler for consistent settings change handling.
  • Misc:
    • Simplified code in ProviderDetail by removing redundant auto-refresh logic.

This description was created by Ellipsis for 3b8a4fe. You can customize this summary. It will automatically update as commits are pushed.

@urmauur urmauur added this to the v0.6.6 milestone Jul 31, 2025
@urmauur urmauur self-assigned this Jul 31, 2025
@urmauur urmauur requested a review from Minh141120 July 31, 2025 06:47
@urmauur urmauur moved this to QA in Jan Jul 31, 2025
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to 3b8a4fe in 44 seconds. Click for details.
  • Reviewed 27 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. web-app/src/routes/settings/providers/$providerName.tsx:133
  • Draft comment:
    Removal of the auto-refresh useEffect for non-predefined providers is clear. Ensure that the global event handler now properly triggers model refreshes, as this change eliminates the 10-second polling. The change looks intentional and simplifies the logic.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_bHJrkbHwDExSAejU

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Contributor

@louis-menlo louis-menlo left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@urmauur urmauur merged commit 59a17d4 into release/v0.6.6 Jul 31, 2025
20 of 21 checks passed
@urmauur urmauur deleted the fix/loop-auto-refresh-model branch July 31, 2025 07:07
@github-actions github-actions bot modified the milestones: v0.6.6, v0.6.7 Jul 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants