Skip to content

⬆️ Update runners and prek checks#349

Merged
burgholzer merged 10 commits intomainfrom
prek
Feb 2, 2026
Merged

⬆️ Update runners and prek checks#349
burgholzer merged 10 commits intomainfrom
prek

Conversation

@denialhaag
Copy link
Member

@denialhaag denialhaag commented Feb 1, 2026

Description

This PR updates the CI to use macos-15 instead of macos-14 and windows-2025 instead of windows-2022. It furthermore updates .pre-commit-config.yml to make use of prek's priority feature.

Checklist:

  • The pull request only contains commits that are focused and relevant to this change.
  • I have added appropriate tests that cover the new/changed functionality.
  • I have updated the documentation to reflect these changes.
  • I have added entries to the changelog for any noteworthy additions, changes, fixes, or removals.
  • I have added migration instructions to the upgrade guide (if needed).
  • The changes follow the project's style guidelines and introduce no new warnings.
  • The changes are fully tested and pass the CI checks.
  • I have reviewed my own code changes.

@denialhaag denialhaag self-assigned this Feb 1, 2026
@denialhaag denialhaag added continuous integration Anything related to the CI setup dependencies Pull requests that update a dependency file pre-commit labels Feb 1, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 1, 2026

Warning

Rate limit exceeded

@burgholzer has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 54 minutes and 55 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

GitHub Actions workflows updated to newer runner versions (macos-15, windows-2025, ubuntu-slim). Pre-commit hooks restructured into a priority-driven configuration with new tools (bibtex-tidy, prettier) and reordered execution flow. Project configuration adds repository-review ignore rule.

Changes

Cohort / File(s) Summary
GitHub Actions Runner Updates
.github/workflows/ci.yml, .github/workflows/release-drafter.yml, .github/workflows/templating.yml
Updated runner environments across CI and release workflows: macos-14→macos-15, windows-2022→windows-2025, ubuntu-latest→ubuntu-slim in respective jobs.
Pre-commit Configuration Restructuring
.pre-commit-config.yaml
Reorganized hooks into explicit priority-driven groups (Fast validation, Second-pass, Final checks); added bibtex-tidy and prettier with configuration; replaced nb-clean with validate-pyproject-schema-store; reordered Python tooling (ruff-format priority 1, ruff-check priority 2, blacken-docs adjusted) with explicit dependencies and sequencing constraints.
Project Configuration
pyproject.toml
Added "PC170" to [tool.repo-review] ignore list with rationale comment regarding rST file deprecation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 With runners fresh and hooks in line,
Priority flows make CI divine,
Ubuntu-slim, macOS refined,
Pre-commit paths perfectly aligned! ✨
Modern tooling, clean and bright,
Our CI pipeline shines so right! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title partially captures the main changes (updating runners and pre-commit checks) but uses 'prek' without clear explanation, making it slightly vague for unfamiliar readers.
Description check ✅ Passed The description covers the main changes and includes most checklist items, but has marked as skipped/skipped several template sections (tests, documentation, changelog, migration) without explaining why.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch prek

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Feb 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 24

🤖 Fix all issues with AI agents
In `@webpage/app/edgeCollector.tsx`:
- Around line 62-68: The Add button currently lacks an explicit type which can
cause accidental form submission; update the JSX button in edgeCollector.tsx
(the button that calls itemAdded(fromRef.current?.value ?? "",
toRef.current?.value ?? "")) to include type="button" so it behaves as a
non-submitting control in embedded forms and prevents unintended form submits.
- Around line 31-38: In itemRemoved, guard against findIndex returning -1 to
avoid accidentally splicing the last element: compute the index from
newItems.findIndex((item) => item[0] == fromVertex && item[1] == toVertex) and
if the index is -1 simply return (or do nothing), otherwise call
newItems.splice(index, 1) and proceed to update state; this change ensures the
function (itemRemoved / items / newItems) only removes the intended edge when it
exists.

In `@webpage/app/edgeDisplay.tsx`:
- Around line 15-18: The interactive <div> using onClick should be made
keyboard-accessible: replace the <div> with a semantic <button type="button">
(preserving the className and onClick handler that calls onClick?.(fromVertex,
toVertex)), or if you must keep a non-button element add role="button",
tabIndex={0}, and an onKeyDown handler that triggers the same
onClick?.(fromVertex, toVertex) for Enter and Space; also ensure you include any
necessary aria-label or title for screen readers and preserve the cursor-pointer
styling when onClick is defined.

In `@webpage/app/graphView.tsx`:
- Around line 127-136: The Cancel and OK buttons in graphView.tsx (the elements
using onClick handlers doCancel and doOk) lack explicit type attributes and can
accidentally submit surrounding forms; update both button elements to include
type="button" to prevent implicit form submission (add the type attribute to the
button with onClick={doCancel} and to the button with onClick={doOk}).
- Around line 72-78: There’s a state update happening during render: the
conditional call to setUploadMode when upload is truthy must be moved into an
effect; replace the inline if (upload && !uploadMode) setUploadMode(true) with a
useEffect that depends on [upload, uploadMode] and calls setUploadMode(true)
inside the effect when upload && !uploadMode, similar to the existing useEffect
that sets isClient via setIsClient.
- Around line 1-4: Replace the static import of Graph from "react-graph-vis"
with a dynamic import using next/dynamic and { ssr: false } so the module is
only loaded on the client; update the import statement (currently importing
Graph) to use next/dynamic and remove the server-side import. After switching to
dynamic import, remove the isClient state variables and their setters (useState
references named isClient) and delete the conditional rendering that checks
isClient before rendering the Graph component so the Graph is rendered directly
on the client-only component. Ensure all references to the old Graph import are
unchanged apart from the new dynamic import symbol to locate the component
usage.

In `@webpage/app/legal/page.tsx`:
- Around line 23-27: Replace the placeholder href="#" used in the two anchor
elements for the legal link with the actual route (e.g., "/legal") and, where
appropriate, indicate the current page state (for example by adding an
aria-current="page" attribute or a "current" class) on the anchor that
represents the active view; update the anchors in the component containing the
Image element and the text "Legal Information" so both point to "/legal" and
include the accessibility/current-page marker.
- Around line 17-22: The anchor element with target="_blank" pointing to
"https://www.cda.cit.tum.de/research/quantum/" (the link text "More on our
Work") must include rel="noopener noreferrer" to prevent tab-nabbing; update
that <a> element (the one with class "text-gray-900 font-medium hidden
md:block") to add rel="noopener noreferrer" alongside the existing target
attribute.

In `@webpage/app/page.tsx`:
- Around line 73-79: The anchor element with href="#" (the <a ...> that wraps
the Image and "MQT QUBOMaker: Pathfinder" text) uses a placeholder URL which is
bad for accessibility; change the href to a real route (for example "/") if it
navigates to the site root, or replace the anchor with a semantic button element
if it triggers a non-navigation action, and ensure any click handler uses
next/link or programmatic navigation consistently with the surrounding Next.js
routing.
- Around line 81-88: The external anchor in page.tsx that uses target="_blank"
(the JSX element with attributes target="_blank",
href="https://www.cda.cit.tum.de/research/quantum/" and className="text-gray-900
font-medium hidden md:block") must include rel="noopener noreferrer" to prevent
tab-nabbing; update that anchor element to add rel="noopener noreferrer"
alongside the existing attributes (the surrounding JSX includes the Image
component) so the link follows security best practices.
- Around line 114-134: The three action buttons (the one that toggles showInfo
via setShowInfo, the "Change Graph" button that calls setDoUpload, and the
download button that invokes download(..., settings.toJson())) need explicit
type="button" attributes to avoid accidental form submission when this component
is used inside a <form>; update the JSX for each button element to add
type="button" alongside the existing props.

In `@webpage/app/pathPosIs.tsx`:
- Around line 16-20: Replace the interactive <div> with a semantic <button
type="button"> (or if you must keep a non-button element, add role="button" and
keyboard handlers) so keyboard users get native support; in the JSX where
onClick?.(path, position, vertex) and the className string are used, change the
element tag to button, keep the onClick handler calling onClick?.(path,
position, vertex), preserve the className and rounded styling, and ensure
type="button" is set to avoid form submission behavior.

In `@webpage/app/pathPosIsCollector.tsx`:
- Around line 78-87: The button lacks an explicit type which can cause
unintended form submission when nested in a <form>; update the button element
that calls itemAdded (the button using pathRef, posRef, and vertexRef and the
onClick that invokes itemAdded(...)) to include type="button" so it will not act
as a submit button.
- Around line 39-46: In itemRemoved (function itemRemoved) guard against
findIndex returning -1 before calling splice: compute the index with
newItems.findIndex(...) and only call newItems.splice(index, 1) when index !==
-1 (otherwise do nothing or early return); this prevents splice(-1, 1) from
removing the last element and ensures the correct item is removed.

In `@webpage/app/settings.ts`:
- Around line 307-322: The code checks shareNoEdges and shareNoVertices only for
non-empty length but never uses their contents, so either use their values as
the path_ids for the constraints or change them to explicit boolean flags;
update the constraint generation in the block that pushes into constraints
(referencing shareNoEdges, shareNoVertices, constraints, and nPaths) to: if the
intent is to restrict specific paths, set path_ids to the arrays' numeric IDs
(e.g., shareNoEdges.map(id => id) / shareNoVertices.map(id => id)), otherwise
replace shareNoEdges/shareNoVertices with boolean properties (e.g.,
forbidSharedEdges, forbidSharedVertices) and check those booleans to add a
constraint with path_ids generated from nPaths as currently done.
- Line 137: The local variable "clone" is never reassigned after initialization
in the Settings-related code; change its declaration from let to const so the
line that creates a new Settings instance (e.g., "let clone = new Settings();")
becomes a const declaration to reflect immutability and improve clarity.
- Around line 247-249: The parseInt calls inside the toJson() serialization
(e.g., edges: this.exactlyOnceEdges.map((pair) => pair.map((val) =>
parseInt(val)), and the other similar mappings) need an explicit radix to avoid
octal/ambiguous parsing; update every parseInt(...) in toJson() to parseInt(...,
10) (apply the same change to the other occurrences referenced: the mappings at
lines around 258-260, 269-271, 291-293, 301-302) so all integers are parsed
using base 10.
- Around line 161-174: The encodingToString method can return undefined for
values other than -1,0,1,2; update encodingToString to handle unexpected
encoding values by adding an explicit branch that either throws a clear Error
(e.g., "Unknown encoding: <value>") or returns a safe default string; locate the
encodingToString method in the settings class and add the explicit throw or
default return after the existing cases so JSON output never becomes undefined.
- Line 285: Remove the debug console.log statement by deleting the call to
console.log(map.keys()); in settings.ts (the debug output referencing the
variable map). Ensure no other stray debug logs remain in the surrounding
function or method where map is used (search for console.log occurrences near
the map usage) so production code does not emit this information.
- Around line 53-59: The setGeneralSettings method reads settings[0..2] without
checking length, which can assign undefined to boolean properties (see
setGeneralSettings, clone, loop, minimizeWeight, maximizeWeight). Fix by
validating the settings array before use: either assert settings.length >= 3 and
throw a clear error, or safely assign defaults when entries are missing (e.g.,
use Boolean coercion or fallback values) so loop, minimizeWeight and
maximizeWeight always receive booleans; update the method to perform this
check/fallback before mutating the cloned instance.

In `@webpage/app/titledTextbox.tsx`:
- Around line 16-22: The label and input in the TitledTextbox component are not
associated for accessibility; generate a stable id (use React's useId() or a
prop) inside the TitledTextbox component, set the input's id to that value and
set the label's htmlFor to the same id (and import useId from React if used), or
alternatively wrap the input in the label—update the JSX where label and input
are rendered to use the matching id/htmlFor pair so screen readers and form
controls are linked.

In `@webpage/app/toggle.tsx`:
- Around line 24-27: Add an explicit type="button" to the button element to
prevent it defaulting to submit, and change the dynamic Tailwind class so full
class names are emitted (don’t interpolate partial class names). Specifically,
in the JSX for the button that uses handleToggle and getIsToggled(), set
type="button" and compute the background class as getIsToggled() ?
"bg-slate-100" : "bg-white" (concatenate that full string into className)
instead of using bg-${...} interpolation.

In `@webpage/app/toggleBag.tsx`:
- Around line 26-30: The effect that resizes the states array in the component
uses useEffect but only lists items in its dependency array, so changes to the
all flag won't retrigger the length check; update the useEffect dependency array
to include all (i.e., useEffect(..., [items, all])) so the check and the
setStates(Array(...)) call run whenever items or all change, keeping states in
sync with items.length + (all ? 1 : 0).
- Line 56: The dynamic Tailwind class `grid-cols-${cols}` in the ToggleBag
component won't be emitted by Tailwind; replace it by computing the layout via
an inline style or a mapping instead: remove the template literal class on the
div that currently reads `className={\`grid grid-cols-${cols} gap-4\`}` and
apply a stable className like `"grid gap-4"` plus a style prop
`gridTemplateColumns: repeat(cols, 1fr)` (or, if you prefer, map numeric `cols`
to explicit Tailwind classes like `grid-cols-1`/`grid-cols-2`/`grid-cols-3`) so
the grid columns render correctly; update the element containing `cols`
accordingly.

@denialhaag denialhaag requested a review from burgholzer February 2, 2026 02:17
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @.pre-commit-config.yaml:
- Around line 50-55: The exclude regex for the pre-commit hook with id
"disallow-caps" is imprecise because the pattern ".*.zip" uses an unescaped dot,
matching any character before "zip" (e.g., "fooXzip"); update the exclude value
to escape the dot so it matches literal .zip (for example use a pattern that
contains "\.zip" or "\.zip$") to ensure only files ending with ".zip" are
excluded; adjust the exclude field in the disallow-caps hook accordingly while
keeping the other exclusions intact.

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: Lukas Burgholzer <burgholzer@me.com>
@burgholzer burgholzer enabled auto-merge (squash) February 2, 2026 12:04
@burgholzer burgholzer merged commit cd7da02 into main Feb 2, 2026
15 of 16 checks passed
@burgholzer burgholzer deleted the prek branch February 2, 2026 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

continuous integration Anything related to the CI setup dependencies Pull requests that update a dependency file pre-commit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants