Skip to content

Conversation

@clementb49
Copy link
Collaborator

@clementb49 clementb49 commented Jan 25, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Fixed message-history rendering so newly added message blocks display correct prefixes, content, and suffixes, improving insertion-point and append behavior.
  • Chores

    • Raised minimum Python requirement and CI/runtime targets from 3.12 to 3.14.
    • Updated build packaging inclusions and dependency layout to align with the new runtime.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 25, 2026

Walkthrough

The PR bumps Python from 3.12 to 3.14 in CI and project metadata, updates packaging include lists, and refactors basilisk/gui/history_msg_text_ctrl.py so append_prefix, append_content, and append_suffix are partialmethod wrappers that receive new_block_ref, absolute_length, and content without explicit self passing; an explicit suffix call was added for user blocks.

Changes

Cohort / File(s) Summary
CI workflow
\.github/workflows/build_app.yml
Windows CI image changed from cpython-3.12-windows-${{ matrix.arch }} to cpython-3.14-windows-${{ matrix.arch }}-none.
Project metadata & packaging
pyproject.toml
Python constraint/target bumped from >=3.12,<3.13 / py312 to >=3.14,<3.15 / py314; zip_include_packages and related include lists expanded/restructured (added packages like _pyrepl, docstring_parser, pathlib*, opentelemetry, tenacity, string, sysconfig, etc.).
GUI function call refactor
basilisk/gui/history_msg_text_ctrl.py
Replaced partial usage with partialmethod for append_prefix, append_content, append_suffix; updated display_new_block calls to omit explicit self and pass new_block_ref, absolute_length, and content; added an explicit append_suffix(new_block_ref, absolute_length) after user content handling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title directly and accurately reflects the main change: updating the project to Python 3.14, as evidenced by version bumps across three files (.github/workflows/build_app.yml, basilisk/gui/history_msg_text_ctrl.py, and pyproject.toml).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

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

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.

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: 3

🤖 Fix all issues with AI agents
In @.github/workflows/build_app.yml:
- Line 25: Update the python-version value to include the libc specifier by
changing the string used in the python-version key from
"cpython-3.14-windows-${{ matrix.arch }}" to "cpython-3.14-windows-${{
matrix.arch }}-none" and ensure the matrix.arch entries expand to the
uv-required tokens (use "x86_64" for 64-bit and "x86" for 32-bit) so the
workflow uses the correct platform identifiers.

In `@basilisk/gui/history_msg_text_ctrl.py`:
- Around line 169-171: The partials used to build block renderers in
HistoryMessageTextCtrl are created with functools.partial (which doesn't bind
self) and later invoked without a self argument, causing a TypeError at runtime;
fix by either converting those partials to functools.partialmethod or by passing
self explicitly when calling them (e.g., ensure the call sites that reference
append_* partials pass self as the first argument), and update both the
occurrence around append_suffix/new_block.request.content and the similar
occurrence at the other location flagged (line ~180) so methods like
append_suffix and the other append_* partials are invoked with an explicit self
or are defined with partialmethod.

In `@pyproject.toml`:
- Line 10: The pyproject.toml declares requires-python = ">=3.14,<3.15" but
wxPython has no cp314 wheels yet, so change the Python requirement to a version
that is supported (for example requires-python = ">=3.13,<3.14") in
pyproject.toml to avoid installing on 3.14; update the requires-python entry
and, if you prefer to support 3.14 later, add a TODO comment in pyproject.toml
or your release notes to revisit when wxPython publishes cp314 wheels.

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 `@pyproject.toml`:
- Line 149: The dependency list includes the private module name "_pyrepl",
which is internal and may change; remove "_pyrepl" from the dependency list in
pyproject.toml or make its inclusion conditional/optional (e.g., only add it
with a python_version >= 3.13 marker or as an optional extra) and document why
it's required if you decide to keep it; update references that rely on "_pyrepl"
so they fall back gracefully when the module isn't present.
♻️ Duplicate comments (1)
pyproject.toml (1)

10-10: wxPython cp314 wheel availability remains a blocker.

This concern was already raised in a previous review. wxPython currently lacks cp314 wheels on PyPI (only supports up to cp313), which will cause installation failures with the requires-python = ">=3.14,<3.15" constraint. Consider waiting for wxPython cp314 wheels or targeting Python 3.13 until then.

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: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
basilisk/gui/history_msg_text_ctrl.py (1)

159-162: Remove explicit self argument — partialmethod auto-binds it.

At line 161, self is still being passed explicitly to append_suffix. With partialmethod, self is automatically bound when called on an instance. This will cause argument misalignment and a runtime TypeError.

Note that line 171 correctly calls self.append_suffix(new_block_ref, absolute_length) without the explicit self.

🐛 Proposed fix
 		if not self.IsEmpty():
 			absolute_length = self.append_suffix(
-				self, new_block_ref, absolute_length
+				new_block_ref, absolute_length
 			)

@clementb49 clementb49 merged commit dd165cb into master Jan 25, 2026
11 checks passed
@clementb49 clementb49 deleted the featPy314 branch January 25, 2026 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants