Skip to content

Keep GitHub items under their requested repository and clear all warnings - #194

Merged
glenn-jocher merged 2 commits into
mainfrom
keep-requested-github-item-url
Sep 23, 2026
Merged

glenn-jocher merged 2 commits into
mainfrom
keep-requested-github-item-url

Conversation

@glenn-jocher

@glenn-jocher glenn-jocher commented Sep 23, 2026

Copy link
Copy Markdown
Member

Follow-up to #193. GitHub's GraphQL API answers a renamed repository under its new name: repository(owner: "zeit", name: "next.js") { issueOrPullRequest(number: 1) { url } } returns https://github.com/vercel/next.js/pull/1. Lite used that answered URL as the item's identity, so it no longer matched the reference that asked for it:

  • Ambiguous candidates skipped their check. A candidate in a renamed repository no longer matched its group, so likelyGitHubItems treated it as a certain item. It was shown regardless of the 30-day activity check. main already did this for a Git remote that still uses an old name; Find GitHub items across wrapped rows and every session repository #193 extended it to every repository the session names.
  • Explicit links showed twice. The placeholder under the printed URL stayed as well as the answered item.

check_github_items now builds the returned URL from the owner, repository and number it asked about, and takes only the kind (pull or issues) from GitHub's __typename. The GraphQL query stops asking for url. GitHub still redirects the old name, and fixing the kind (a #N asked as an issue that is a pull request) works as before. No TypeScript changes. Version 0.0.58.

Validation

  • A temporary live test, not committed, ran check_github_items against real GitHub:
    • zeit/next.js/issues/1 came back as zeit/next.js/pull/1 (merged, last updated 2019), so the 30-day check now drops it as a candidate.
    • ultralytics/lite/issues/193 came back as ultralytics/lite/pull/193.
    • A repository that doesn't exist still returns an unanswered placeholder, and a number that doesn't exist is still dropped.
  • cargo fmt --check and cargo test pass. Clippy reports no new warnings.
  • bun run check and bun test (32) pass.

🛠️ PR Summary

Made with ❤️ by Ultralytics Actions

🌟 Summary

GitHub issue and pull-request lookups now retain the repository name Lite was asked about, while the session-launch payload and build configuration were also updated.

📊 Key Changes

  • GitHub lookups no longer use the URL returned by GraphQL; Lite builds the item URL from the requested owner, repository, and number, using __typename to determine whether it is an issue or pull request.
  • spawn_session now receives session settings in a nested launch object, and App.tsx sends that shape.
  • Set Vite’s chunk-size warning limit to 800 KB, updated the CI Bun install command to allow scripts, and bumped Lite to 0.0.58.
  • Refactored several Rust conditionals and platform-specific checks.

🎯 Purpose & Impact

  • References to renamed repositories keep the identity of the requested item, so activity checks and explicit-link matching use the same URL. The session-launch payload change updates the internal frontend-to-Rust command interface.
📋 Skipped 1 file (lock files, generated, images, etc.)
  • src-tauri/Cargo.lock

… to 0.0.58

GitHub answers a renamed repository under its new name, so a candidate in one lost its group and skipped
the 30-day activity check, and an explicit link in one showed twice. The item keeps the name it was asked
by and takes only its kind from GitHub's answer.
@UltralyticsAssistant UltralyticsAssistant added bug Something isn't working fixed Bug has been resolved labels Sep 23, 2026
@UltralyticsAssistant

Copy link
Copy Markdown
Member

👋 Hello @glenn-jocher, thank you for submitting a ultralytics/lite 🚀 PR! This automated message confirms your contribution was received, and an Ultralytics engineer will assist with the review. To ensure a seamless integration of your work, please review the following checklist:

  • Define a Purpose: Clearly explain the purpose of your fix or feature in your PR description, and link to any relevant issues. Ensure your commit messages are clear, concise, and adhere to the project's conventions.
  • Synchronize with Source: Confirm your PR is synchronized with the ultralytics/lite main branch. If it's behind, update it by clicking the 'Update branch' button or by running git pull and git merge main locally.
  • Ensure CI Checks Pass: Verify all Ultralytics Continuous Integration (CI) checks are passing. If any checks fail, please address the issues.
  • Update Documentation: Update the relevant documentation for any new or modified features.
  • Add Tests: If applicable, include or update tests to cover your changes, and confirm that all tests are passing.
  • Sign the CLA: Please ensure you have signed our Contributor License Agreement if this is your first Ultralytics PR by writing "I have read the CLA Document and I sign the CLA" in a new message.
  • Minimize Changes: Limit your changes to the minimum necessary for your bug fix or feature addition. "It is not daily increase but daily decrease, hack away the unessential. The closer to the source, the less wastage there is." — Bruce Lee

For more guidance, please refer to our Contributing Guide. Don't hesitate to leave a comment if you have any questions. Thank you for contributing to Ultralytics! 🚀

@UltralyticsAssistant UltralyticsAssistant left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🔍 PR Review

Made with ❤️ by Ultralytics Actions

Reviewed the GitHub item lookup changes and version bump. The returned URL preserves the queried repository and number while using GitHub’s typename for the item kind, fixing identity matching without an evident regression. LGTM.

📋 Skipped 1 file (lock files, generated, images, etc.)
  • src-tauri/Cargo.lock

- Linux and Windows builds warned about the unused `app` in load_codex_server and an unneeded `mut` on the
  shell command: platform branches now use cfg! so each compiles everywhere.
- Clippy: nested ifs become let-chains, and spawn_session takes its interface-decided fields as one
  camel-cased SessionLaunch instead of 22 arguments.
- Vite's 500 kB web heuristic becomes a startup budget sized for a chunk Lite loads from disk.
- CI allows Bun's install script, which newer npm otherwise warns it skipped.
@glenn-jocher glenn-jocher changed the title Keep a GitHub item under the repository name it was asked by Keep GitHub items under their requested repository and clear all warnings Sep 23, 2026
@UltralyticsAssistant
UltralyticsAssistant dismissed their stale review September 23, 2026 06:11

Superseded by new review

@UltralyticsAssistant UltralyticsAssistant left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🔍 PR Review 2

Made with ❤️ by Ultralytics Actions

Since Review 1, the PR adds warning cleanup, groups the session launch payload under launch, and adjusts the CI install flag and Vite chunk budget. The GitHub identity fix remains intact, and the additional changes show no concrete behavioral regression. LGTM.

📋 Skipped 1 file (lock files, generated, images, etc.)
  • src-tauri/Cargo.lock

@glenn-jocher
glenn-jocher merged commit e631ee4 into main Sep 23, 2026
6 checks passed
@glenn-jocher
glenn-jocher deleted the keep-requested-github-item-url branch September 23, 2026 06:12
@UltralyticsAssistant

Copy link
Copy Markdown
Member

Huge thanks to @glenn-jocher — this PR has been merged! Preserving the requested repository in GitHub lookups and updating the session-launch payload brings these parts of Lite’s workflow into alignment.

“Great things are done by a series of small things brought together.” — Vincent van Gogh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working fixed Bug has been resolved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants