Repository navigation
Conversation
Record the agreed plan for rewriting the GUI as native PySide6 widgets, plus the parked outline for the sortition-algorithms 0.12 bump it defers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There are no tests in this repo - they were deleted along with run-tests.yml when the app moved to the sortition-algorithms library. The Eel to Qt port needs a safety net before any logic moves, so: - add pytest-qt and mypy dev dependencies, and PySide6-Essentials - add a justfile with `just test` and `just check` - restore .github/workflows/run-tests.yml, across three OSes - commit fixtures: the categories CSV plus a generated pool of people that matches it, and a pool too small to fill a panel Two pre-existing problems had to be fixed to get `just check` green: poetry-style `authors` in pyproject.toml (RUF200) and formatting in hook-mip.py. script.py gets a temporary mypy override - every eel.* call is an attr-defined error - which goes away with script.py. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Characterisation tests, written from reading script.py rather than from running it, recording the exact calls the app makes into the browser for each user action. The recorded sequences become the specification for the view protocol the session layer will be extracted onto. Two things fell out of writing them: - the wording of the library's reports is "Number of features found", not "Number of categories" as the static HTML claims - running a selection twice appends the second panel to the first in the download link, because CSVStringDataSource writes into a StringIO that is never truncated. Documented in a test named after the bug; the port fixes it by writing to a file the user chooses. The gsheet tests run against a fixture-backed fake data source, so they need no credentials and no network. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every piece of logic in script.py called eel.some_js_function() directly, which is why none of it was testable. Those calls become methods on view protocols, and the logic moves into strat_app/sessions/ where it imports neither eel nor Qt. script.py drops from 610 lines to 155 - it is now just the eel entry points - and keeps working through eel_views.py, which implements the protocols with the old eel calls. Both files go in step 6. What changed rather than moved: - panel size is an int, not a string that is parsed and echoed back. The update_eel loop guard, panel_size_str/panel_size_num and the string validation all go, in both handlers. Free text parsing now lives in the eel entry point, where the free text box is. - log entries are held as text or as library RunReports, and rendered by the view. The session layer produces no markup. The one hand-written bit of HTML - the several-selections warning - becomes an IMPORTANT report line, so it renders bold rather than bold-plus-italic. - output buffers are reset before each run, fixing the bug where a second selection was appended to the first. - SettingsHolder takes the settings path, so tests do not touch $HOME. - dropped the assignments to data_source.selected_tab_name and remaining_tab_name: the library overwrites both from its own GSheetTabNamer when writing, so they had no effect. The step 1 characterisation tests are ported to the recording view, and joined by tests for the settings holder, the log, and the eel shim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Native PySide6 widgets mirroring the old page: same two tabs in the same order, same headings, step numbering and wording, the same collapsible advanced settings and detailed log. No HTML page, no browser, no JS. The two deliberate changes from the plan are here: the panel size is a QSpinBox clamped to the feasible range, so an impossible size cannot be typed, and the output is saved through a file dialog rather than pulled through a data: URI. Three things only visible by looking at the rendered window: - Qt reads a single & in a tab title as a keyboard shortcut and swallows it, so the tabs read "CSV file input _output" until escaped - a checkable QGroupBox does not collapse if you hide its rows one by one - the rows need to be in a widget of their own - without a scroll area the tabs squash their contents rather than scrolling, and the advanced settings rows silently overlap Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A leximin run over a real pool takes minutes. With eel that only wedged the gevent loop and the browser carried on drawing; in Qt it freezes the window, which looks exactly like a crash. The session layer gets a TaskRunner it hands slow work to, and knows nothing else about threads. Tests use SynchronousRunner, so they stay predictable - every existing session test passed unchanged through this restructure, which is what said the restructure was right. The app uses QtTaskRunner, which runs the work on a QThread and delivers the callbacks back on the GUI thread through a queued connection. Everything that touches a widget runs in a callback. Off-thread now: the selection itself on both tabs, and reading from and writing to Google Sheets. The log is the one thing worker threads write to directly, through a QueuedLogView, because "Requesting people data from tab X ..." is only useful while it is happening. Feedback during a run: - an indeterminate QProgressBar, since 0.11.5 reports no phases to make a real one out of. The run buttons are disabled while it shows. - the library's user_logger is bridged into the detailed log, so trial numbers and algorithm choices appear as they happen rather than all at once at the end. The library both logs those lines and puts them in the report, so the detailed log renders reports with include_logged=False and they do not appear twice. The progress reporter seam from the plan is in place: sessions hold a progress_reporter that is always None until the library gains one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
strat_app/__main__.py builds the QApplication, shows the window and installs the library's live log into the detailed log for the lifetime of the app - a windowed build has no stdout for the default handler to write to. main.py is the plain script PyInstaller prefers to build from. --self-test builds the window, pumps the event loop and exits 0. CI runs it against the built artefacts in the next step: "could not load the Qt platform plugin" only ever shows up in a packaged build, and today CI builds three binaries and never runs any of them. The end to end tests drive the real window with the real library: choose both files, run, save both outputs, and check the selected and remaining CSVs partition the input pool exactly. The gsheet flow runs the same way against a fixture-backed data source, so it needs no credentials - it proves the tab, the worker and the wiring, not gspread. Also pins the pre-commit ruff to the version in the dev dependencies. They had drifted eight minor versions apart and disagreed about the same file, so `just check` passed while the commit hook refused it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
script.py, eel_views.py, the web directory and the Eel dependency all go,
along with the mypy overrides that existed only because eel's JS bridge
made every call an attr-defined error, and the pytest warning filters for
the deprecations eel emitted as it imported.
Removing Eel takes gevent, gevent-websocket, greenlet, zope.event,
zope.interface, future and importlib-resources with it. The app no longer
needs a separately installed Chrome, a localhost TCP port, a gevent
patched event loop, or a CDN it silently relied on for Bootstrap and
jQuery - which meant the CSV workflow needed an internet connection.
The plan's checkable aim holds:
rg -i '<html|innerHTML|eel\.|bootstrap' -> 0 matches
README rewritten for the Qt app, and its test badge fixed - it pointed at
a workflow name and a branch that have not existed for some time. The
docs TODO about the output tab names is answered while I am here, since I
had to read that part of the library to remove the code around it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces the `python -m eel script.py web ...` build wrapper with strat-select.spec, which covers all three platforms: onefile on Linux and Windows, onedir plus an app bundle on macOS with target_arch=x86_64 as before. Qt modules we do not use are excluded; PySide6-Essentials had already kept QtWebEngine out of the wheel set. CI now runs each artefact it builds with --self-test, which it has never done - three binaries were built and none of them ever started. The flag builds the window and runs a small selection, so it covers both the Qt plugin loading and the scientific stack, which is where packaged builds actually fail. On Linux it runs under xvfb rather than the offscreen platform, so it exercises the xcb plugin - the one that goes missing. It paid for itself on the first run. Two real faults found: - cvxpy lists its own package directory at import on Python 3.12+, which does not exist when the modules are in the archive, so the binary died before showing anything. Fixed with module_collection_mode. Existing releases avoided this only by being built on 3.11. - OSQP fails to import in the bundle. Harmless as it turns out - the library asks for SCS, ECOS or mip, and SCS, CLARABEL and HIGHS all work in the bundle - but worth knowing rather than guessing. Measured on Linux: 157 MB, 4.7 s to start, against 126 MB today. Both inside the spike's limits. An onedir build starts in 2.5 s but takes 235 MB on disk; that is a distribution change, so it is left alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replace the parked outline with a worked plan, checked against tags 0.11.5 and 0.12.10 of the library and against this app's own code. Two of the three risks the outline named turn out not to be risks - GSheetTabNamer predates 0.12 and this app never touched the tab name attributes, and RunReport is unchanged. The two things that do bite were not on the list: mip 1.17 moved CBC into a 181MB cbcbox wheel, and the retry loop moved out of run_stratification so three tests that assert on "Trial number: 1" will fail. Also add llm-working/upstream.md with the issues found in the library while researching. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nothing else changes yet, so that whatever goes red is attributable to the bump alone. Three tests fail, all of them the ones predicted in the plan: they assert on "Trial number: 1", which 0.12 only emits for the legacy algorithm now that the retry loop has moved out of run_stratification. Fixed in the next commit. pandas and scikit-learn drop out of the lock - 0.12 makes them an optional diversimax extra - and cbcbox arrives via mip 1.17. Also exclude the gitignored ignore/ directory from mypy, which otherwise finds a second module named "tests" in the library checkout kept there and refuses to run at all. ruff already skips it by way of .gitignore. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
0.12 moved the retry loop out of run_stratification and into the legacy algorithm's wrapper, so "Trial number: N" is only logged when the legacy algorithm is in use. The default is maximin, so three tests waiting for that line waited for two minutes and then failed. "Using maximin algorithm." is logged as the first statement of find_distribution_maximin, before any solver work, so it arrives just as early and says something the app actually cares about. Both tests still prove what they were written to prove: LogPanel builds its browser with include_logged=False, so a logged line can only reach that widget through the live handler, never through the end-of-run report. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The library's default solver backend is highspy, and it only imports mip
if you ask for a mip backend. Since 1.17 python-mip keeps CBC in a
separate cbcbox wheel that unpacks to 275MB - larger than the rest of
the bundle put together - so excluding both is the difference between a
bundle that shrinks and one that nearly triples.
find_spec("mip") comes back empty in the frozen app, so the library sets
MIP_AVAILABLE to False and never reaches for it.
Linux bundle: 156.4 MiB -> 141.9 MiB. The packaged --self-test runs a
real selection through highspy and passes.
highspy needs no hook of its own, so pyinstallerhooks/ is now empty and
goes away along with hookspath - git cannot track an empty directory, so
leaving it would break a fresh clone.
Also correct the macOS target_arch comment, which blamed python-mip for
a pin that no longer has that reason.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The library accepts more than the packaged app ships. A mip solver backend needs python-mip, which we leave out because it carries 275MB of CBC, and diversimax needs pandas and scikit-learn, which 0.12 made optional and we do not install. Both are values a user can write in their settings file, and neither fails until a run is well under way. Refusing means not storing the settings, so loaded() stays False and both sessions already bail out early - an unsupported value now behaves exactly like a broken settings file, reported next to whatever the user just did. Deliberately a refusal rather than a quiet substitution: swapping the user's solver for a working one would have two machines with the same settings file selecting panels by different methods without saying so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The warning has always told the user "You cannot use the 'Produce a Test Panel' button if you want more than 1 selection", and the button has always let them anyway. Under 0.11.5 that produced a tidy report, because the library raised ValueError and run_stratification caught it. Under 0.12 it raises ConfigurationError, which does not subclass ValueError and so escapes run_stratification entirely - the user gets a bare "Unexpected error during selection" instead. The honest fix is not to let them ask. set_run_enabled used to drive both run buttons, so refusing only the test panel needs the two split apart. set_busy has to remember and restore both independently, or finishing a run hands back a button that was deliberately disabled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two of the three new manual checklist items turned out not to need Google at all - padding every header name with spaces and loading through the session proves the same thing the manual check would have. Doing it found a real gap. 0.12's whitespace fix went into SelectionData.load_people and load_already_selected but not load_features, so a padded respondents tab now loads while a padded categories tab still fails with "Did not find required column name 'category'" - naming a column the user can see spelled correctly. Not worked around here, since that would put library behaviour in the wrong place. Recorded as upstream issue 5, with a test documenting the current asymmetry so it is written down rather than rediscovered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
0.12 adds a ProgressReporter protocol and threads it through every algorithm, so the app can finally say how far through a run is rather than only that something is happening. On a ten minute selection over a real pool that is the difference between "working" and "possibly hung". Split in two. ProgressThrottle in the session layer does the deciding and is Qt-free, so it is tested against a clock the tests control - 1000 updates in, exactly 10 out, no timing flakiness. QtProgressReporter is then only signal plumbing. The throttle earns its place: the library states it does not throttle and calls update() every iteration of its inner loops. Phase changes and the last value of a phase are never dropped, because those are the two a user would notice missing - a lagging label, and a determinate bar stopping at 187/200. Connections are queued, so the library can drive the reporter from the worker thread while the slots run on the GUI thread. That also means nothing a slot does can reach back into the library's call stack. Against the real library a maximin run gives multiplicative_weights with a total of 200 (determinate) followed by maximin_optimization with no total (busy), and the throttle cuts the stream to 55 events. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each step in the plan now says what happened, including the two places the plan was wrong: the bundle saving is 14.5 MiB rather than the "well under 120 MB" guess, because pandas and sklearn compress well inside the archive, and cbcbox unpacks to 275MB rather than the 181MB its wheel suggested. Also records the two bugs found along the way that belong upstream, and that the manual Google Sheets checklist is still outstanding - it needs credentials and a human, and it is the one thing blocking merge. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A QtTaskRunner built as a local goes out of scope the instant the test returns, possibly while its QThread is still winding down - and a QThread destroyed while running aborts the process. That shows up as an occasional SIGABRT somewhere else entirely, which is what one full-suite run did. Now a fixture that calls wait() on teardown, matching test_workers.py and what MainWindow.closeEvent does in the app. The abort was never reproduced on demand, so this is a real hazard removed rather than a proven cause. Noted in the plan along with the pre-existing race in QtTaskRunner._reap to look at if it recurs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
osqp resolves its algebra backend with importlib at runtime, so PyInstaller never sees osqp.ext_builtin. cvxpy probes every solver it knows about when it builds its installed list, and without the backend that probe logs "RuntimeError: No algebra backend available!" where users see it. We solve with SCS, so nothing actually breaks - but users on the eel build report it as an error, and a few hundred KB buys its way out of it. https://github.com/orgs/osqp/discussions/657 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GitHub has retired the macOS 13 image, so the macOS build job matched no runner and sat queued until it was cancelled or hit the 24 hour timeout. The `-intel` images are the x86_64 runners that replace it, which answers the TODO that was sitting on this job. The note about target_arch no longer names python-mip as the reason, since the spec stopped bundling mip and already explains what an arm64 build would take. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Moving the build over to strat-select.spec took all three release jobs out of the workflow with it, so a tag push built three artefacts and attached none of them to the release. They come back renamed to match the build jobs, on Linux runners since all they do is move a file from an artefact to the release, and with the macOS one pointing at the zipped app bundle - its artefact holds the unpacked onedir build too, which cannot be a release asset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Convert the app to use QT to make a desktop app, instead of Eel (which is no longer maintained).
Also upgrade sortition-algorithms and add a progress bar.