perf(engine): cols is positional, and the order is produced rather than repaired by FBumann · Pull Request #433 · fluxopt/specsolve

FBumann · 2026-07-31T17:44:21Z

The second attempt at #428. Same prize, and this time the invariant costs nothing.

What was wrong the first time

cols holds one row per column and labels are globally dense 0..n-1, so in label order its col column is the frame's own row number — 0.32 GB of index at 40M columns, held for the model's lifetime, plus the work it caused: lp_file sorted cols twice and dense_columns scattered three of its four vectors through it.

#428 bought that order with a sort in the executor. It measured 8–16% off peak and 13–92% onto the build, and was closed with its table.

What produces it for free

polars' streaming engine walks a cross join right-major. In memory it is row-major; streamed it is not:

in-memory   [0, 1, 2, 3, 4, 5, 6, 7]      row-major
streaming   [0, 3, 6, 9, 1, 4, 7, 10]     right-major

So folding the coordinate product backwards emits it in declaration row-major order — which is label order — at no cost whatsoever. It is the same joins in the other sequence. Verified at four arities, then on all six bench cases: every variable and constraint label frame now arrives in exact label order.

Three changes carry it:

_coordinate_product folds in reverse, then projects the declared column order back (the fold reversed that too, and test_a_frame_cross_joins_its_dim_tables_and_carries_their_ordinals pins it).
_factored's second cross join gets the same swap — head outer, survivors inner.
The bounds join carries maintain_order='left': +12 ms on a 10M-row join, against the ~110 ms sort it replaces at that size.

Numbers

bench.run --cases dispatch nodal transport profiled --sizes m l --repeat 3, back to back, load 1.87 on 8 cores.

	peak	build	wall
`dispatch/l` lp	−20.8%	+9.4% (noise)	−7.4%
`transport/l` lp	−14.5%	+1.0% (noise)	−0.8%
`profiled/l` lp	−10.9%	−9.2%	−8.2%
`nodal/l` lp	−10.2%	−4.7%	−5.8%
`dispatch/l` highs	−5.6%	+10.0%	−13.1%
`profiled/l` highs	−3.6%	−10.1%	−12.1%

Better peak than #428 achieved (−20.8% against −16.4%, because there is no sort buffer either), build within ±10% instead of +13–92%, and wall improves nearly everywhere. The remaining build regressions are maintain_order on dispatch, whose wall is 13% better regardless.

The part that needs care

The order is a property of polars, not of this package. A label is arithmetic on ordinals rather than a position, so a change in polars' cross-join emission would not corrupt a label — it would silently stop the frame being sorted while cols is still read positionally, and every bound would attach to the wrong variable.

So _in_label_order verifies rather than trusts: is_sorted is a linear scan over a column the frame already holds, with a sort behind it as the correctness floor, expected never to run.

The test asserts the bounds land on the right columns by checking against the label frame, which knows independently. Two dims, because one dim is a scan and cannot be out of order — a single-dim model passes whatever the product does. Distinct bounds per coordinate, because a permuted frame still holds the right multiset. Mutation-checked: removing either guard fails it, and takes test_shift_semantics_are_positional_not_lexicographic with it.

For the duckdb branch

ModelTables.cols losing a column is a contract change, so #399 needs its half at the next merge — duckdb's label relations are ordered by SQL, which guarantees nothing without an ORDER BY. test_both_engines_write_the_same_lp_file compares the two engines' LP output byte for byte and fails immediately if it is forgotten, which is why that half belongs in the merge rather than in a PR of its own.

Checks

745 passed, ruff + pyrefly (0 errors) + mkdocs build --strict clean.

🤖 Generated with Claude Code

…an repaired `cols` holds one row per column, and labels are globally dense 0..n-1 — so if it arrives in label order its `col` column is the frame's own row number. 0.32 GB of index at 40M columns, held for as long as the model is, plus the work it caused: `lp_file` sorted it twice and `dense_columns` scattered three of its four vectors through it. The first attempt bought that order with a sort in the executor and cost 13-92% of the build (#428, closed with its numbers). This produces the order instead. **polars' streaming engine walks a cross join right-major.** Folding the coordinate product backwards therefore emits it in *declaration* row-major order, which is label order, at no cost — it is the same joins in the other sequence. The factored path's second cross join gets the same treatment, and the bounds join carries `maintain_order='left'`, which is +12 ms on a 10M-row join against the ~110 ms sort it replaces. on a quiet machine, minimum of three repeats peak build wall dispatch/l lp -20.8% +9.4% -7.4% transport/l lp -14.5% +1.0% -0.8% profiled/l lp -10.9% -9.2% -8.2% nodal/l lp -10.2% -4.7% -5.8% dispatch/l highs -5.6% +10.0% -13.1% That order is a property of a *dependency*, not of this package, and a label is arithmetic on ordinals rather than a position — so a change in polars would not corrupt a label, it would silently stop the frame being sorted while `cols` is still read positionally. `_in_label_order` therefore verifies rather than trusts: a linear scan over a column the frame already holds, with a sort behind it as the correctness floor. The test uses two dims because one dim is a scan and cannot be out of order, and distinct bounds per coordinate because a permuted frame still holds the right multiset. Removing either guard fails it.

coderabbitai · 2026-07-31T17:44:32Z

Warning

Review limit reached

@FBumann, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 9 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fd101f41-63f6-453a-a738-51c00ec94bcc

📥 Commits

Reviewing files that changed from the base of the PR and between 31ba48c and ee26d52.

📒 Files selected for processing (6)

src/lpspec/relational/engines/polars/compiler.py
src/lpspec/relational/engines/polars/executor.py
src/lpspec/relational/engines/polars/labels.py
src/lpspec/relational/sinks/tables.py
src/lpspec/relational/sinks/writers/lp_file.py
tests/test_relational.py

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.}

`test_relational.py` is the only conflict and it is textual — main appended `test_dense_columns_does_not_edit_the_model_it_projects` (#427) where this branch appended the positional-cols test. Both kept. They belong together. #427 pins that the projection must not rewrite the model in place, and it is *this* change that makes that reachable: with `cols` positional, `dense_columns` hands out views of the frame rather than the scatter's fresh arrays, so `nan_to_num` had to stop being in-place. The guard was written against a hazard this branch creates. 754 passed, 23 of them Gurobi's, ruff + pyrefly + mkdocs --strict clean.

FBumann · 2026-07-31T18:07:57Z

Rebased on #421 and #434, and re-measured against the base as it now stands. The peak numbers hold; the build column moved and is worth reading honestly.

	peak	build	wall
`dispatch/l` lp	−20.5%	+16.5%	−5.6% (noise)
`profiled/l` lp	−12.0%	−6.7%	−6.4%
`transport/l` lp	−11.5%	+8.7%	+0.5% (noise)
`nodal/l` lp	−10.8%	−10.1% (noise)	−12.3%
`dispatch/l` highs	−5.6%	−9.4% (noise)	−34.1%
`profiled/l` highs	−3.1%	−8.8%	−12.4%

Peak is better on all eight rungs, 3–20%, every one outside its floor. Wall is better or noise everywhere. Build is the mixed one: −10% to −9% on nodal/profiled, +8.7%/+16.5% on transport/dispatch where maintain_order='left' on the bounds join is the cost — and both of those rungs are flat-or-better on wall regardless.

The conflict was textual, in tests/test_relational.py only — main appended #427's test_dense_columns_does_not_edit_the_model_it_projects where this branch appended the positional-cols test. Both kept, and they belong together: #427 pins that the projection must not rewrite the model in place, and this change is what makes that reachable, because with cols positional dense_columns hands out views of the frame rather than the scatter's fresh arrays.

tables.py auto-merged. #421's dense_rows/row_blocks and this PR's dense_columns are the row and column halves of one argument and sit next to each other without touching.

754 passed, 23 of them Gurobi's; ruff + pyrefly (0 errors) + mkdocs build --strict clean.

read-the-docs-community · 2026-07-31T18:08:26Z

Documentation build overview

📚 lpspec | 🛠️ Build #33857446 | 📁 Comparing ee26d52 against latest (393a910)

🔍 Preview build

1 file changed

± changelog/index.html

Five commits, one of which changes the contract: #433 drops `col` from `cols`, because it holds one row per column in label order and a row's position is therefore its solver index. **That order is free on polars and is not on duckdb.** polars gets it from the emission order of a cross join and `_in_label_order` only verifies it; SQL promises no order at all, so the duckdb engine asks for one outright — `ORDER BY var_label` on the bounds query, and the `col` column gone with it. The parity gate is what caught the unmigrated engine, exactly as it was meant to: `test_both_engines_write_the_same_lp_file` failed byte for byte before the duckdb half was written, on a change that is silent everywhere else — a positional frame in the wrong order builds a model whose every bound belongs to another variable, and solves. `_frames` in that test sorted `cols` by a column that no longer exists. It now compares them row for row, which is a stronger claim than it was making: with `col` present, sorting hid any disagreement about *order* and only compared the multiset. Two conflicts, both the same shape — this branch had already moved a thing main was still editing in place. The frame schema lives in `sinks/tables.py` here (#407) rather than in the polars executor, so main's `_COLS`/`_DTYPES` fold into `sinks.COLS` (which loses `col` too); and `_predicate_dims`/`_free_prefix` live in `plan.py` here (#403), so only main's new `_in_label_order` is taken from its copy of `labels.py`. 776 passed on polars, 775 passed / 5 skipped on duckdb, ruff + pyrefly (0 errors) + mkdocs --strict clean.

#439) * perf(sinks): a solution is a vector, not a vector beside its own index #414 made the read-back positional — a declaration owns a contiguous run of labels, so its share of a solver vector is a slice. The frame the solvers hand back never caught up: `_labelled` still builds `arange(count)` alongside the values, as a key for the join #414 deleted. Nothing reads it. That is 8 bytes a column, held for as long as the result is: 80 MB at `dispatch/l`, 96 MB at `fleet/l`, 0.96 GB at the `2xl` rung — and the same again in rows for the duals. It is the argument #433 used to take `col` off `cols` ("the column would be the frame's own row number"), on the other side of the solve. So both solvers return a `pl.Series` of the values, and `Result` holds one. `_read_back` slices it directly instead of reaching through a column name. No benchmark: nothing here is faster, an allocation is simply not made, and its size is `count * 8` rather than something to measure. `test_a_solver_hands_back_a_vector_and_not_an_index` pins it across both solvers, because — as with #433 — the numbers are identical either way and nothing about the answer can see the difference. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(engine): a solver vector that does not span the model is refused Reading a solution back is positional, so a vector of the wrong length is an answer about a *different* model rather than a short answer about this one. Refused where the solver hands it over rather than where it is read. The objective comes back from the solver directly, so a `Result` built on a broken vector reports a plausible number and fails only if someone asks for a coordinate — and then from inside polars, about a frame the caller never asked for. A vector that is too *long* was not refused at all: every declaration's slice still lands. `gurobi._duals` already did this for its own half; this is the same check where both solvers pass through, and it is one comparison against a count. Raised by CodeRabbit on #439. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ug made (#441) * docs(bench): re-measure the published tables, and reverse a conclusion The committed results were taken at `ef8558c` — version `0.0.0a27`, before the package was even renamed. Eight perf changes have landed since (#408, #412, #413, #414, #415, #434, #421, #433), so every table on the page understated the engine, and one paragraph drew a conclusion the numbers no longer support. Re-run in one sitting on an idle machine: the ladder, the density sweep and the scaling run, so all three files describe the same tree. Parity gate passes on all six cases (`fleet` to 4.6e-16) before anything is timed. **What moved**, against the previously published numbers, `l` rung, our arm: wall -10% to -39% on nine of twelve rungs, peak -2% to -21% on nine. **What got worse, and is not buried.** `dispatch` build is 10-12% slower — that is `maintain_order='left'` from #433, which buys -21% peak on the same rung and is the trade that PR argued for. `fleet/highs` peak is up 8.2%, outside its 1.0% floor, and I have no explanation for it. **A conclusion is reversed.** `## Absurd sizes` read 1.07x peak at 120M variables and concluded that our memory headroom "is a small-and-sparse-model property, not a scaling one". Peak is now 0.73-0.83x across four orders of magnitude. Two things moved it and only one is us: #433 took ~20% off this case, and the earlier reading came from a run *that section itself flagged as noisy* — it recorded linopy at 38.5 s and 106.2 s for the same rung. The old number was a conclusion drawn across the noise floor. The section now says so rather than quietly presenting a better one. **`profiled` is no longer the case in the ladder we lose** (1.09x -> 0.74x through the highs sink), and `fleet/lp` has crossed on wall (1.03x -> 0.90x). Both paragraphs are rewritten to say what changed rather than to delete the sentences that admitted the losses. Two numbers still go against us and are still named: `profiled` 1.31x on wall through the LP sink, `fleet` 1.47x on peak. Not touched: `## Sink capabilities` still says Gurobi's row wants a spike before it is relied on. #418 shipped a real Gurobi sink since, so that caveat is probably obsolete — but exercising it is not this change, and asserting capabilities I have not run is how that table would come to lie. * docs(bench): re-measure at 98f382d, and correct a claim the harness bug made Every number on the page now comes from one run at `98f382d` on a clean tree, and the page says so — the fingerprint hash is what makes a published number traceable to the code that produced it, and the previous one recorded `782f76b-dirty`, which identifies nothing. **The `highs` comparison was not like for like, and it flattered us.** #438 found that linopy's `to_highspy()` was being timed *with* variable naming, which neither of our solver sinks does — 82% of its hand-off, and memory as well as time. Corrected: highs sink, l rung was now dispatch peak 0.87x 1.46x fleet peak 0.76x 1.10x profiled peak 0.82x 1.27x transport peak 0.75x 1.10x So the summary changes from "ahead on both, on all six" to **ahead on wall on five of six, and behind on peak on four of six through `highs`**. HiGHS's own dense model dominates both arms there and our COO triples are what is left. The `lp` sink barely moved, because that comparison was already like for like, and it is where the representation actually pays: ahead on peak on four of six. `profiled` is the case in the ladder we lose, on both sinks (1.27x, 1.07x). Said plainly rather than left to the tables. The scaling section's reversal stands and is re-measured: peak 0.68-0.78x from 10k to 120M variables, against the 1.07x this page used to conclude the advantage "runs out" from. That section now also says it is the LP sink, and therefore not evidence for the `highs` peak row.

…one (#445) * chore(main): release 0.0.1-alpha.58 (#443) Co-authored-by: fluxopt-release[bot] <267260463+fluxopt-release[bot]@users.noreply.github.com> * docs(bench): the engine comparison at e42b9a0, and a docs clobber undone Two things, and the second one is a fix for damage I did. **The numbers.** Measured at `e42b9a0` on a clean tree, all six cases, both sinks, three repeats; noise floors are in `bench/results/engines.jsonl` beside them. duckdb is **1.3-3.4x slower on build across all twelve rungs and heavier on eight of them** — lighter only on `fleet` and on `nodal` through the solver. Objectives stay bit-identical; the parity gate passes at 0.0 relative. The three places that claimed otherwise are corrected: the engine's own docstring (which said it "earns its keep at the top of the ladder", 0.55x on `dispatch/l`), ROADMAP Track 4, and `bench/duckdb-spike.md`. **The reason matters more than the ratio, and it is partly ours.** Five optimisations landed on the polars engine while this one sat on a branch. Then `cols` went positional (#433), which is *free* there — the order falls out of a cross join's emission order — and costs an `ORDER BY` here, roughly doubling `dispatch/l`'s build the day it landed. A shared contract can be cheap for one engine and not the other, and the gap widens with neither engine regressing. That is a structural fact about shipping two, and the docs now say it. **The clobber.** The previous merge ran `git checkout origin/main -- docs/` to drop a stale results file and took the whole directory with it, including the ARCHITECTURE module map, the engine diagram and hard rule 2 — all of which this branch adds and main does not have. `test_every_module_is_documented_somewhere` caught it: `relational/engine.py` had no entry because the entry had been deleted. Restored from this branch's own side, with the Track 4 correction reapplied on top. 780 passed on polars, 779 passed / 5 skipped on duckdb, ruff + pyrefly + mkdocs --strict clean. --------- Co-authored-by: fluxopt-release[bot] <267260463+fluxopt-release[bot]@users.noreply.github.com>

…sing it (#446) #433's argument, at the third reader. Every path in `Labeller.frame` hands back a label-ascending frame and two of them verify it, so `_read_back` sorting again re-established an order the frame never lost. It is not the seconds that matter. Sorting moved a full copy of the coordinates — strings included — at the moment the solver's own model is still resident, which is the worst point in the process to allocate one. Reading every variable of `dispatch/l` back: time over the built model process peak sort 0.0624s +681.0 MB 1.86 GB no sort 0.0006s +0.2 MB 1.19 GB fleet/l 0.0673s -> 0.0029s +337.3 MB -> +0.2 MB 2.20 -> 1.87 GB nodal/l 0.0216s -> 0.0004s +225.6 MB -> +0.1 MB 1.07 -> 0.84 GB Those peaks carry no solver: in a real solve the saving lands on top of HiGHS's own copy of the model, which is resident throughout. Every declaration of every bench case at `m` and `l` — 66 variables and constraints across six models, primal and dual — returns a frame identical to the sorted one. `_in_label_order` now names both readers that take the order on trust. It justified itself by `cols` alone, and a reason that lists one of two is how the guarantee gets weakened for the other. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Brings #399 up to `ce0fb3d` — #443, #446, #447, #444 and #448. **#448 replaced the custom runner with a pytest suite.** `bench/run.py`, `_run_case.py` and `compare.py` are gone; this branch's `run.py` carried the duckdb arm and main's `conftest.py` already has it, so the deletion is taken and nothing is lost. `report.py` keeps main's `.json` reader and this branch's `--arms`, without which the engine comparison cannot be tabulated at all. **#446 needed care.** It stopped `_read_back` sorting, because every labelling path already yields a label-ascending frame and two of them verify it. True of polars. Not true of a SQL relation: two of duckdb's three label paths are views over a cross join and SQL promises no order, so inheriting the unsorted read-back would attach primal values to the wrong coordinates — silently, on a solve. duckdb's label registry now asks for the order on the way out, where only a caller reading a solution pays it. That is the third time today a change free on polars had to be bought here: `cols` going positional (#433), the read-back (#446), and the registry. The asymmetry is the branch's most useful finding and is written up in `bench/duckdb-spike.md`. 781 passed on polars, 780 passed / 5 skipped on duckdb, ruff + pyrefly (0 errors) + mkdocs --strict clean.

Every sink reads the matrix a row range at a time, so one handed an unordered matrix orders it — a second pass over a finished frame, and a second copy of it while the solver's own model is resident. Ordering it in the build happens inside the pipeline that is already materialising the rows. A lone term gets the order from its *join*: the row frame is in label order since #433, and `maintain_order='left'` keeps it — half the cost of sorting the result on `dispatch/l` (0.068 s against 0.129 s). Several terms are several ordered runs stacked, and runs are not an order, so those sort. Only the lone term asks the join for anything, because a stack pays the sort regardless and the join's guarantee is then dead weight. `_in_row_order` states the claim after the stack, the way `_in_label_order` does for the labels: the concatenation is ordered by construction — ascending row ranges of ordered shares — but polars cannot see that through a `concat`, so it is verified with a linear scan and then flagged. The sort behind it is the correctness floor and is expected never to run. This is what makes the coordinate test in the previous commit pay. Measured together on `transport/l`, in-process and paired: today: sort(row,col) + check build 0.975s handoff 0.488s total 1.463s coordinate test alone build 0.466s handoff 0.956s total 1.422s both build 0.686s handoff 0.531s total 1.217s Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… and the matrix leaves in row order (#487) * perf(polars): two group_sums of one variable collide only where the coordinates meet #408 established that two fragments can share a solver column only if they carry the same variable. Sharing a variable is necessary and not sufficient, and the assembly treated it as both — so `transport`'s balance, `group_sum(f, by=to) - group_sum(f, by=from)`, sorted every nonzero in the model to discover that nothing repeated. At `l` that is 0.20 s of a 0.69 s constraint, and the answer is always no. Two fragments of one variable reach the same row only if the operators that moved their labels send them there. A fragment now records those operators, so mappings that differ **only in a coordinate** ask the question they actually pose: is there a line whose two ends are one bus? That is a scan of the `line` table — forty rows at `l`, against 12.6M nonzeros. Everything else answers yes. A shift on one side and not the other, a reduction that left them over different dims, a product that broadcast one wider — each moves a label in a way this does not model, and the cost of being wrong is a matrix holding one cell twice, which an LP reader sums and a solver handed duplicates may do either way. `may_share_a_column` also answers *no* for distinct variables, which is #408's half of the same question, so the assembly asks one thing rather than two. **Not a win on its own** — it is measured with the row-ordering that follows, because dropping the sort alone hands the sink an unordered matrix and it sorts instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(polars): the matrix leaves the build in row order Every sink reads the matrix a row range at a time, so one handed an unordered matrix orders it — a second pass over a finished frame, and a second copy of it while the solver's own model is resident. Ordering it in the build happens inside the pipeline that is already materialising the rows. A lone term gets the order from its *join*: the row frame is in label order since #433, and `maintain_order='left'` keeps it — half the cost of sorting the result on `dispatch/l` (0.068 s against 0.129 s). Several terms are several ordered runs stacked, and runs are not an order, so those sort. Only the lone term asks the join for anything, because a stack pays the sort regardless and the join's guarantee is then dead weight. `_in_row_order` states the claim after the stack, the way `_in_label_order` does for the labels: the concatenation is ordered by construction — ascending row ranges of ordered shares — but polars cannot see that through a `concat`, so it is verified with a linear scan and then flagged. The sort behind it is the correctness floor and is expected never to run. This is what makes the coordinate test in the previous commit pay. Measured together on `transport/l`, in-process and paired: today: sort(row,col) + check build 0.975s handoff 0.488s total 1.463s coordinate test alone build 0.466s handoff 0.956s total 1.422s both build 0.686s handoff 0.531s total 1.217s Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

… never repair it (#534) * style: inline comments become docstrings, and one helper The explanatory inline comment is retired: what narrated code is gone, what carried a fact — a measured number, an issue reference, a library subtlety — now lives in the nearest docstring, and the one expression complex enough to deserve a name (the null-counting matrix aggregate, written twice) is the _summed_shares helper. Kept inline: pragmas with their reasons, #: attribute docs on constants, and section banners. CLAUDE.md states the rule so it holds for new code too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(engine): ask the data, not the declarations — and produce order, never repair it Interleaved A/B against main (l rungs) had the branch at 2.1-4.7x on build. Four recoveries, none of them the deleted machinery: - the coordinate product folds in reverse again (#433): the streaming engine walks a cross join right-major, so the product arrives in label order and everything downstream can read position instead of sorting. - labelling is one path: row-major position by arithmetic, a linear is_sorted verify, a single-key sort only when a join actually lost the order. The unconditional multi-key sort was 0.26 s of a 0.73 s build. - order-bearing joins say so: maintain_order='left' on bounds and mask joins (+12 ms at 10M rows, against ~110 ms to sort afterwards), 'left_right' on a lone term's placement. - the matrix aggregate runs only when a linear adjacency probe finds a repeated cell, and the writer stamps set_sorted('row') — the order ModelTables already promises — so the streaming engine groups adjacent runs instead of hashing: 0.14 s against 0.89 s at 10M nonzeros. After: transport/l 1.10x, storage/l ~1.3x, dispatch/l 2.2x (its vacuous where pays one single-key sort main's factored-prefix machinery avoided; that trade is the point of #520 and is documented, not resurrected). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* refactor(engine): one labelling path, one terminal aggregate The relational lane carried three pieces of machinery whose only job was to avoid work: a `keyed`/`label_dims`/`mapping` triple on every term fragment so the assembly could skip its terminal aggregate, three routes through the labeller depending on how much of the coordinate product a mask left, and an inner-vs-left join choice for the names a where is certain of. Each was correct and each was a second thing a reader had to hold. They are gone. The matrix and the objective always aggregate — the one place duplicates were ever meant to collapse — labels are always the sorted product numbered from the next free index, and a mask always left-joins. Order is now established where it is needed (`sort('var_label')`, `sort('row')`) rather than inherited from a cross-join fold and verified after the fact. The LP writer's constraint section follows: the interleaving integer-key sort becomes a group into a list per row and an explode, with the two sorts that make the bytes reproducible stated on either side of the join. Same models, same labels, same bytes. * refactor: state the matrix order once, drop the probe-then-name checks Three smaller pieces of the same kind of machinery. `ModelTables` now promises its sinks a matrix sorted by `(row, col)`, which the engine establishes once; the solver hand-off and the LP writer read that instead of each re-sorting a frame that already had the order. `check_one_row_per_coordinate` asked a cheap boolean first and only paid for the group-by on the path about to raise, and the single-valued check rode in the caller's aggregate through a scratch-column protocol between two modules. Both now just ask the question they mean. Also: `plan.children` never descended into `At`, so `divisor_parameters` could not name a divisor under one; `Builtin.value_kwargs` was a field no helper declared; `_helper_sum` had three branches doing one thing. * refactor(engine): drop a divisor guard the assembly can no longer reach Every constraint's share of the matrix now arrives from one concat of term pieces, so it always carries `coeff` — the column-presence test in front of the null check was a leftover from the branch that could return early. * chore(bench): profile the labelling function, not the class it used to be on * fix(engine): close the holes the simplification opened, keep its shape The objective now refuses an undefined divisor the way constraints do, on both lanes — its aggregate was the one place a null coefficient could still be read as a zero. The duplicate-row check's count column is named '#rows', which no dim can be, where 'n' crashed every build of a model with a dim of that name. The polars floor rises to 1.36, where `explode(empty_as_null=)` exists — the LP writer relies on it. The constraint-section docstring now cites the within-group order guarantee #109 rests on, and a test docstring still naming the deleted Labeller says labels.frame. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(engine): one streaming pass per share, already in order at assembly The divisor-null count rides the aggregating group_by instead of forcing the unaggregated stack eager first: each constraint share and the objective are one lazy streaming pipeline again, and the check reads a count the aggregate already carried. And the assembly-time sort is gone — each share leaves sorted and owns the next run of rows, so ascending shares concatenate into an ascending whole, and the sort was copying the model's largest frame at the peak of the build to reorder nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(engine): one streaming pass per share, already in order at assembly (#528) * perf(engine): validation asks its cheap questions in one pass (#529) * perf(engine): validation asks its cheap questions in one pass Data validation probes a parameter once — any duplicate coordinate, any unknown label — and pays the passes that name offenders only on a path about to raise; a healthy build of a large model no longer runs the group_by that was the single most expensive step of its build. And an explicit dimension index is collected once, with the single-valued check and the coordinate grouping both reading that one collect, instead of each re-scanning a source that may be a file (#273). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(lp): the writer's peak is bounded again (#530) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * style: inline comments become docstrings, and one helper (#532) The explanatory inline comment is retired: what narrated code is gone, what carried a fact — a measured number, an issue reference, a library subtlety — now lives in the nearest docstring, and the one expression complex enough to deserve a name (the null-counting matrix aggregate, written twice) is the _summed_shares helper. Kept inline: pragmas with their reasons, #: attribute docs on constants, and section banners. CLAUDE.md states the rule so it holds for new code too. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * perf(engine): ask the data, not the declarations — and produce order, never repair it (#534) * style: inline comments become docstrings, and one helper The explanatory inline comment is retired: what narrated code is gone, what carried a fact — a measured number, an issue reference, a library subtlety — now lives in the nearest docstring, and the one expression complex enough to deserve a name (the null-counting matrix aggregate, written twice) is the _summed_shares helper. Kept inline: pragmas with their reasons, #: attribute docs on constants, and section banners. CLAUDE.md states the rule so it holds for new code too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(engine): ask the data, not the declarations — and produce order, never repair it Interleaved A/B against main (l rungs) had the branch at 2.1-4.7x on build. Four recoveries, none of them the deleted machinery: - the coordinate product folds in reverse again (#433): the streaming engine walks a cross join right-major, so the product arrives in label order and everything downstream can read position instead of sorting. - labelling is one path: row-major position by arithmetic, a linear is_sorted verify, a single-key sort only when a join actually lost the order. The unconditional multi-key sort was 0.26 s of a 0.73 s build. - order-bearing joins say so: maintain_order='left' on bounds and mask joins (+12 ms at 10M rows, against ~110 ms to sort afterwards), 'left_right' on a lone term's placement. - the matrix aggregate runs only when a linear adjacency probe finds a repeated cell, and the writer stamps set_sorted('row') — the order ModelTables already promises — so the streaming engine groups adjacent runs instead of hashing: 0.14 s against 0.89 s at 10M nonzeros. After: transport/l 1.10x, storage/l ~1.3x, dispatch/l 2.2x (its vacuous where pays one single-key sort main's factored-prefix machinery avoided; that trade is the point of #520 and is documented, not resurrected). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * fix(language): expansion sees the borrowed names, and once is enough Three findings from the #533 review, addressed here since the merge brought that code onto this branch: - extend() now threads its known_variables into expand_piecewise, so an extension whose piecewise links borrow a model variable validates against the namespace it will run in instead of being refused with "'p' not found". Regression test proves the failure without the fix. - A validated Model keeps the expansion its own validation built, keyed by the namespace it was expanded against; expand_piecewise returns it rather than building — and fully re-validating — a second Model. Every piecewise consumer (build, extend, lowering, typeset) drops from two validation passes to one. - The SPEC piecewise example is checked again: doctest skip becomes wrap=piecewise, so a typo'd key in the example fails CI instead of shipping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor: sweep what the deletion left behind A four-angle cleanup pass (reuse, simplification, efficiency, altitude) over this branch's diff: - TermFragment.variable had lost its last reader when may_share_a_column went; the field, its threading through four constructors, and the test asserting it go too. - _group_fragment and _at_fragment had converged to the same four lines read in opposite directions; one _remap_fragment states the join once. - parameter_join's maintain_order knob had one setting at every call site; the join now always keeps order and says why. - The VariableDefined branch spells its null-reads-as-false through _falsy_if_null like every other mask site. - _build_variable projects (var_label, lb, ub) before collecting, so the dim strings and bound aliases stay in the lazy pipeline instead of materialising to be dropped — and its verify-then-sort goes through labels.in_position_order rather than restating it. - The matrix's sorted flag is stamped where the order is established (executor assembly), not by one sink; the LP writer's per-chunk full-matrix filter becomes ModelTables.matrix_block, the binary-search slice row_blocks already used, so both chunked readers share one answer to which entries a row range owns. - The piecewise expansion cache is owned entirely by expand_piecewise — write, read, and key in one module, misses repopulate — with Model keeping only the slot. - _build_constraint's docstring described the intermediate always- aggregate design; it now states the shipped probe-then-collapse. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(engine): materialise the masked suffix, not the product — and one key per line Codspeed on the branch still showed -30..-60% against main: memory on small models, wall on the lp sink. Interleaved A/B (median of 5 paired subprocess runs) reproduced both and profiling put them in two places. Labelling a variable whose mask cannot see its leading dims walked the full coordinate product through the mask join: +177 MB transient and 17 ms for sector/m's one masked variable, where main's factored path paid for the suffix. That split returns, as one guarded branch of the one labels.frame path: a mask that reads none of the leading dims removes the same coordinates under every one of their values, so the survivors are a rectangle — rank the suffix set, cross-join the head product on the left (the streaming engine emits the right side fastest, which is label order), and the label is arithmetic again. Restrictions and masks that read the first dim still count the slow way. The writer's group-into-list-and-explode paid per list element: 3x main on sector/m emit. The one-key-per-line union sort returns — header, placeholder, terms, footer each pick a slot in a chunk-relative integer key and one single-column sort interleaves them — now fed by ModelTables.matrix_block, so the terms arrive as a pre-sorted slice rather than a per-chunk scan. A/B after (branch/main wall, peak): sector-m-lp 1.01/1.08 was 1.81/1.31; sector-s-lp 1.09/1.02; nodal-m-lp 0.98/1.03; fleet-m-lp 1.09/0.98; storage-m-lp 1.09/1.00; sector-m-highs 1.06/1.09. dispatch-m-highs stays 1.24 wall: its mask reads both dims, so no prefix is free and the counted path pays its sort — the documented residual of deleting the static machinery, now confined to that class. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(engine): a mask that joins restricts by semi-join, and order survives The mask path carried a caveat: joins keep the frame's order, except a value join over the lazy cross product, which is why a mask reading the first dim forced labelling's verify into a real sort (dispatch 1.24x). The caveat dies instead of being documented harder. A predicate is a function of only the dims it reads, so it is evaluated once over their product and the full product is semi-joined against the truth set — the mask's parameter columns never touch the full product, and a semi-join leaves left order alone, so 'every frame is produced in order; sorts are verified fallbacks' now holds without exceptions. Pointwise predicates (dimension comparisons, constants) stay a plain filter, detected by _predicate handing the carrier back unchanged. predicate_dims moves from labels.py to the compiler: the semi-join and the factored prefix now read one answer to 'which dims does a where read'. Interleaved A/B vs main after: dispatch-m-highs 1.19x wall / 0.97x peak (was 1.24/1.02), dispatch-m-lp 1.13/1.00, sector-m-lp 0.99/0.92, sector-s-lp 1.05/0.99, fleet-m-lp 1.06/0.92 — peaks now at or below main everywhere measured; dispatch's residual is the per-declaration probe passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(bench): the walltime rung ladder grows an l, on the evidence it was waiting for The job's own comment deferred l until a series showed whether m resolves what we want to see. #520 answered: fixed per-declaration overhead reads as -5..-10% at m and amortizes to within ±4% at l, where peak memory — the product claim — moves the other way (0.88-0.99 of main). The two rungs disagree, which is what the metered rung exists to catch. Timeout widened to fit the second rung. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(engine): three measured claims outlived the code they described #550 dropped the matrix's `row` column, and two justifications went stale with it. `_in_row_order` said a sink that finds the sorted flag missing reorders the whole matrix, and the assembly said the flag is what lets a sink's streaming `group_by` walk adjacent runs (0.14 s against 0.89 s at 10M nonzeros) — no sink reads the flag now, or the column, because `row_blocks` is a pure slice. The verify still earns its scan for a different reason: `_row_starts` scatters one run per row by value, so a column out of order silently loses spans. It is the correctness floor of the CSR layout, and `set_sorted` goes, having no reader left. `chunking` said one pass spends the budget, which was true for exactly as long as the writer's group-into-list rendering survived; the union sort came back with `EMIT_BUDGET`, so it is two again. `HANDOFF_BUDGET` said the same thing in its own words. * refactor: three duplications, each of a rule rather than of code `ModelTables` spelled the row-span rule twice — once in `row_blocks`, once in `labeled_blocks` — including the `budget=None` case, whose whole argument is that the caller decides. It is now `_spans`, which also absorbs `_row_chunks_by_nonzeros`: one private method answers which ranges a block reader walks, rather than two stacked on `chunking.ranges`. `Model.model_validate` and `model_validate_json` were the same try/except with the same paragraph above each. The paragraph is the point — why pydantic's own error will not do, and why `__init__` may not be wrapped the same way — so it moves to `_in_our_tree`, where the two doors cannot come to answer differently. `labels._factored` had its own copy of verify-then-sort. `in_position_order` now keeps the witness column instead of dropping it, so all three orderings in the lane go through it; the one caller that has no further use for the witness projects it away, which is also how `cols` gets exactly the three columns a sink reads. Net six lines, and that is the wrong measure: what went is three places a rule was written twice. * refactor(engine): the shift's presence rule gets a name, and keeps its narrow key Measured before touching it. Always-full-width presence — deleting `presence_dims` and `_widen` — costs 1.15-1.21x of build on an acyclic ramp at 8760x200, over two interleaved A/B runs, 12 of 14 paired rounds against it. The masked control, where the change is inert, came back 1.021x with pairs straddling 1.0, so that is the branch and not the harness. `presence_dims` stays. What was available was the reading. The presence half of `_translate_fragment` was an if/elif whose two arms did unrelated things — travel a set that exists, or invent one that did not — with the key threaded through three assignments. It is now `travelled_presence`, which states its three cases and carries the argument for each, including why the narrow key is worth its subtlety. Worth recording separately: no case in `bench/` takes this branch at all. `storage` is the ladder's only `shift` and it is cyclic, so every change to shift presence — #546 and #551 included — has been landing unmeasured. * chore(deps): give back the polars floor the LP writer stopped needing `polars>=1.36` was raised for `explode(empty_as_null=)`, which the group-into-a-list-and-explode constraint section passed. That rendering measured 3x the union sort and was reverted; the keyword is now nowhere in the tree, so the floor was buying nothing. Floors are load-bearing here — the bare-install job re-resolves at `--resolution lowest-direct` precisely so a declared lower bound is a claim rather than decoration — and CLAUDE.md says raise one when you rely on a version's behaviour, not to whatever is current. * chore: retrigger CI * perf(lp): put back the presort this branch dropped by accident The constraint stream's `matrix.sort('row', 'col')` is redundant for correctness — the union sort below subsumes it and the bytes are identical without it — and it is not redundant for speed: polars' sort exploits ordered runs, so the union is faster on pre-ordered input than on unordered. That was measured before (+12% emit on `transport/l`, the note #517 rewrote) and this branch lost it anyway: the group-into-a-list rendering had no such line, and when the union sort came back the presort did not come with it. dispatch/m lp write 0.127 s -> 0.079 s (main: 0.089) sector/l lp write 0.186 s -> 0.171 s (main: 0.146) `dispatch/m` now writes faster than main. Same bytes — the reproducibility check still writes one model three times identically, and test_lp_text passes. * perf(labels): the factored cross join had its operands the wrong way round `_factored`'s comment claimed the head product belongs on the left because the streaming engine emits the right side fastest. Measured, it is the other way: with head on the left the labels came out unordered and `in_position_order` sorted the whole variable frame — a million rows on every build of `dispatch/m` and `sector/l`, which is most of why this branch read slower than main. dispatch/m build 0.055 s -> 0.036 s (main: 0.031) sector/l build 0.259 s -> 0.217 s (main: 0.213) Nothing about the labels changes: they are arithmetic on the ordinals, and the verify was already there to catch exactly this. What the verify could not do is say it was firing — the probe that found it counts how often it sorts, and the answer was one call of three on both cases, every time. The docstring now records that which side cycles is a dependency's detail, that the verify is what makes exploiting it safe, and that the argument it replaces was confidently wrong. * perf(engine): the objective probe reads order before it counts The objective is the one consumer that reads term fragments raw — no placement join re-derives order — so its walk asks the mul join to keep the left side's row order, and the repeated-column probe becomes one linear pass over the ordered stack. The n_unique hash count survives only for a stack that really is unordered (several terms concatenated, or a join that shuffled), where adjacency proves nothing. Ordered-ness is bought, not free: the ordered collect cost +20 ms at 10M rows on dispatch/l against 42 ms of n_unique saved. That is why keep_order is threaded from the objective walk alone rather than defaulted on — sector and storage multiply inside constraints, whose shares are re-ordered by the placement join anyway, and taxing those joins buys nothing. Interleaved A/B against main, build phase: dispatch/l objective 74 -> 58 ms, transport/l 52 -> 38 ms; constraint and variable phases unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(mask): a full-width mask goes back to the direct path, and certainty returns Two reversals, one reachable only behind the other, measured together by a six-arm interleaved A/B (median of paired ratios vs main): sector/m 1.075 -> 1.009, the only delta in the corpus above the harness's ~5% noise floor. Credit to a parallel review session for the attribution and the patches; docstrings and their numbers are this commit's. A mask that reads every frame dim has a truth set as wide as the product itself, so the semi-join built the coordinate product twice to save no width — sector's balance mask is exactly that shape and paid 6.6% of the m pipeline for it. It returns to the plain filter, which keeps order the same way. Behind that, the one-strategy uniformity stops paying: on the direct path the rows a left join keeps are rows the filter then drops, so a name the mask is certain of is inner-joined and a certain variable semi-joined again (_certain_parameters, back with its Or/Not asymmetry stated). Behind a semi-join's key product the two strategies measured the same, which is why uniformity looked free when it landed. parameter_join takes how/maintain_order again; only the bounds ask for order (cols is positional), every other consumer verifies where it reads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(labels): nothing renumbers unless a row was dropped Only a mask or a restriction leaves gaps in the row-major positions. With neither, every coordinate survives and start + position is the label, so the unmasked path stops materialising a position column only to renumber it with a row index and drop it — one arithmetic expression, as before the paths merged, but inside the single labels.frame. Phase-measured, not wall-measured: ~3 ms per unmasked declaration at the m rungs (fleet carries 19 and no where at all), 1.5-2% of build at the l rungs — consistent in sign across cases but below the end-to-end noise floor of the local harness. Landed for the shape as much as the number: the renumber pass existing only where a row was dropped is the honest form of the one-rule labeller. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(compiler): pin the two branches the recovery left untested (#573) 6dc3c24 and 725cd0e are performance-only branches: delete either and every one of the 941 tests still passes, every model still solves to the same answer, and sector quietly gives back the 6.6% the guard recovered. The fallbacks are semantically identical by construction — a full-width mask filters where it would have semi-joined, a certain name is left-joined where it would have inner-joined — so nothing that reads an answer can hold them. Verified as pins rather than as decoration: both new assertions fail at 43a7109 (the branch before the recovery) and pass at 90d87b3. test_compiler.py already exists for exactly this ("the only place query *shape* is asserted", and every property there "can regress while the whole suite still passes"), so its invariant list gains the two entries rather than a new file appearing beside it. test_a_parameter_predicate_needs_a_left_join is renamed: the predicate it compiles is one the mask is certain of, so 6dc3c24 made it an inner join and the name had stopped being true. Its left-join half moves to the same predicate under an Or, where certainty does not survive. Not pinned, for want of an observable: 3a0619e's objective probe. keep_order threads maintain_order into _join_mul, which polars does not print in a query plan, so deleting the linear branch and reverting to n_unique on every build is invisible to a shape assertion as well as to an answer. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(engine): the share is made contiguous before it is probed (#576) #574 asked whether the assembly probes could be skipped by propagating the sortedness labelling already establishes. They cannot: polars clears the sorted flag on both `concat` and `join(maintain_order='left')`, which are the two operations between labelling and assembly, and re-asserting it after a join is the move `set_sorted` cannot check and the verify exists to avoid. Measuring that premise found the real cost, which is not the comparison but the chunking. A streaming collect returns its morsels as chunks -- 104 for dispatch/l's 10M-entry share, 136 for transport/l's 12.6M -- and `shift(1)` pays at every boundary. `is_sorted` does not: 1.18 ms chunked against 1.14 ms flat. In situ the probe cost 28 ms and 35 ms where a synthetic contiguous frame of the same shape said 5 ms, and that 6x gap is the whole finding. Rechunk, then probe: 12 ms and 19 ms including the rechunk. A single combined key probed with `is_sorted` was also tried and is worse (21 ms and 26 ms) -- building the key gathers across the same chunks. The copy is not new work. #550 already requires the matrix to be contiguous or every block's `to_numpy` is a gather, so the assembly rechunks regardless; this moves that copy earlier, and where one declaration owns the whole matrix the later rechunk becomes a no-op. Interleaved A/B against the branch head, 7 rounds, min of 3, two independent runs: dispatch/l wall 0.960 / 0.927 peak 0.944 / 0.942 (-142 MB) transport/l wall 0.960 / 0.978 peak 0.969 / 0.993 fleet/l wall 0.963 / 0.960 peak 0.993 / 0.996 sector/m wall 1.007 peak 1.000 fleet is the shape that does not benefit in peak, as expected: 19 declarations mean 19 small rechunks and a `_stack` that re-fragments to 19 chunks, so its later rechunk still copies. The objective's stack takes the same `concat().collect()` and is deliberately left alone: rechunking it too measured no consistent wall gain and cost dispatch 2% of peak (0.963 against 0.942). Like the two branches #573 pins, this one is invisible to the suite -- a rechunk changes no answer -- so the docstring carries what a future reader needs to not delete the rechunk. The A/B ratios stay here and in the PR: they compare against a branch head, which stops naming anything once #520 merges. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(engine): the recovery commits shed their threading and their twins (#575) No behavior change — every query leaves expression-identical — only shape: - keep_order stops being threaded: product and quotient never read self, so they move into expression()'s walk as closures and capture the flag where it is stated. No signature carries it anymore. - labels.frame's two numbering branches become one skeleton with a single 'dropped' decision, so 'renumber only when a row was dropped' is now the code rather than a docstring claim about two code paths. - The full-width-mask guard turns positive: the semi-join states when it applies and the filter is the fall-through, instead of three negated disjuncts guarding an early return. - The objective probe's nested branches flatten to one 'repeated' answer. - The certainty story is told once, in _predicate, where the decision is made; parameter_join points at it instead of retelling it. Net -22 lines on the PR's diff against main. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

…it (#1616) > **Prompt:** "I dislike inline comments. If we need them, we should probably refactor and have a proper method or sth else thats annotatable. […] Docstrings should be user-facing and not argue about why the method does things a certain way or why the api is like this. Only state facts about the code behaviour and maybe use cases if needed!" > [!NOTE] > The following content was generated by AI. Every docstring and comment under `src/lpspec/` now states what the code does and the invariants a change could break, and drops the design defenses, performance rationale, and benchmark numbers with their `#nnn` refs — that story stays in the PR and git. The one explanatory inline comment (the HiGHS integrality cast) became an annotatable `_integrality` helper. <details><summary>Scope, and what was kept versus cut</summary> All 46 files under `src/lpspec/` reviewed; ~215 docstrings and `#:` attribute-docs edited across nine commits, net ~1,000 lines removed. **Cut** — the "why", per the prompt: - Design and API-choice defenses (`Its own class rather than a DataError because…`, `A tuple on purpose`, `not a knob`, `deliberately`). - Performance rationale and benchmark numbers with their `#nnn` issue references (`79x the CPU (#459)`, `3.4 ms to 1.0 ms (#1589)`, `an order of magnitude (#433)`, …). - Rejected-alternative comparisons and prior-implementation history. - Summaries that opened with `Why …` — rewritten to name the value returned. **Kept** — facts about behaviour: - Caller contracts: `Args`/`Returns`/`Raises`, and what each public verb does. - Behavioural invariants a change could break, stated as facts: the `set_sorted` assertion, the ascending-row requirement, the `-0.0`/`+-0.0` LP parser gotcha, the MPS `UP`-below-zero rule, the `pl.lit` Int32/Int64 key gotcha, the span check, the solver release order, hard rule 2. - Dependency version and bug gotchas: the HiGHS `1.15.1` empty-integrality note, and the one surviving issue ref — `PyPSA/linopy#906`, kept because it is tied to a reproducing error message rather than to a rationale. - All `#:` attribute facts, `:math:` markup, section banners, pragmas, and every code fence and doctest block. **Inline comments:** the only explanatory inline comment in `src/` — the `# kContinuous is 0 …` note in the HiGHS build — moved into an `_integrality(cols)` helper with a factual docstring, per the prompt's "refactor and have a proper method". The rest were section banners and pragmas, which stay. </details> <details><summary>Verification, and what could not be checked</summary> - `ruff check .` passes repo-wide; `ruff format --check src` reports all 46 files formatted. This is the docstring gate (`D`, Google convention) that `pixi run check` enforces. - Confirmed no code fence or doctest block content changed — the diff over `src/` touches docstring and comment prose only, no signatures or logic. - **Not run:** the fuller `pixi run pytest` suite (e.g. `tests/test_doc_examples.py`) — `pixi` and the installed package are not present in this environment. That suite validates code fences, none of which were touched, so the risk is nil, but it is unrun. </details> **Deliberately not done:** the `docs/` site prose, already brought to this standard in #1560 and #1564 — this change makes the in-tree docstrings match that principle. Each commit is typed `chore`: a docstring-only `src/` change is invisible to a changelog reader, matching #1612. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Jms579WrvFgWzFKrNdemer --- _Generated by [Claude Code](https://claude.ai/code/session_01Jms579WrvFgWzFKrNdemer)_ --------- Co-authored-by: Claude <noreply@anthropic.com>

FBumann added area:engine Lowering, IR, relational executor, the sink contract perf Wall time, memory, or I/O cost — not a wrong answer labels Jul 31, 2026

FBumann enabled auto-merge (squash) July 31, 2026 18:09

FBumann merged commit 22bb9c7 into main Jul 31, 2026
4 checks passed

fluxopt-release Bot mentioned this pull request Jul 31, 2026

chore(main): release 0.0.1-alpha.55 #437

Merged

This was referenced Jul 31, 2026

perf(sinks): a solution is a vector, not a vector beside its own index #439

Merged

docs(bench): re-measure at 98f382d, and correct a claim the harness bug made #441

Merged

This was referenced Jul 31, 2026

docs(bench): the engine comparison at e42b9a0, and a docs clobber undone #445

Merged

perf(engine): the read-back reads the label order rather than re-imposing it #446

Merged

FBumann mentioned this pull request Jul 31, 2026

feat: Add a second engine (duckdb) #399

Closed

FBumann mentioned this pull request Aug 9, 2026

perf(polars): two group_sums collide only where the coordinates meet, and the matrix leaves in row order #487

Merged

FBumann mentioned this pull request Aug 10, 2026

perf(engine): ask the data, not the declarations — and produce order, never repair it #534

Merged

This was referenced Aug 11, 2026

The assembly probes re-derive an order labelling already established #574

Closed

perf(engine): the share is made contiguous before it is probed #576

Merged

FBumann mentioned this pull request Aug 12, 2026

refactor: every claim lives where it is read, not in a comment #612

Merged

Navigation Menu

Repository navigation

perf(engine): cols is positional, and the order is produced rather than repaired - #433

perf(engine): cols is positional, and the order is produced rather than repaired#433
FBumann merged 2 commits into
mainfrom
perf/cols-positional

FBumann commented Jul 31, 2026

Uh oh!

coderabbitai Bot commented Jul 31, 2026 •

edited

Loading

Review limit reached

Uh oh!

FBumann commented Jul 31, 2026

Uh oh!

read-the-docs-community Bot commented Jul 31, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

FBumann commented Jul 31, 2026

What was wrong the first time

What produces it for free

Numbers

The part that needs care

For the duckdb branch

Checks

Uh oh!

coderabbitai Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

FBumann commented Jul 31, 2026

Uh oh!

read-the-docs-community Bot commented Jul 31, 2026

Documentation build overview

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Jul 31, 2026 •

edited

Loading