Uh oh!
There was an error while loading. Please reload this page.
✨ Election Architect v0.0.1 — step's half, as one diff - #3087
Draft
edulix wants to merge 257 commits into
Draft
Conversation
Every id in a generated bundle is a version 5 UUID over the event's external_id, the entity kind and the row's external_id. Regenerating an unchanged source therefore produces byte-identical output, so a diff between two runs shows only what the author actually changed; and two events built from different sources never collide, because the event's external_id is mixed into the namespace. it_agrees_with_the_python_it_replaces pins six ids produced by janitor's ids.py. They match on the first run, which is what lets the Rust take over without renumbering any event already generated — including the SEIU1000 bundle. Written out rather than taken from the uuid crate. Its v4 feature pulls getrandom, whose WASM support is version-specific and already pinned elsewhere in this workspace; nothing here needs randomness and it should not acquire a reason to. And the byte layout is the thing that must never change — alter it and every event ever generated renumbers — so it is better read than trusted. sha1 is the only new dependency, pure Rust and already in the lock file. Two details that are easy to get wrong and are now tested: Parts are length-prefixed rather than joined by a separator. Joined by one, ["a/b"] and ["a", "b"] hash alike, which for an area/contest link means two different pairs sharing an id and one silently overwriting the other. An external_id holding a slash is unusual, not forbidden. The prefix counts characters, not bytes, matching the Python. A byte count is the more obvious choice in Rust and would renumber every id derived from a non-ASCII external_id for no gain, since either count is unambiguous. "José-Muñoz" is ten characters and twelve bytes; a test pins it against the Python's answer. Related: sequentech/meta#12769 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The templates supply the platform boilerplate; the source document overrides it.
That split is what keeps client configuration out of the code — a delivery
engineer who needs a different default supplies their own template instead of
patching a tool. Moving the eight .hbs files and the renderer into the shared core
is what lets step-cli and a browser produce the same bundle from the same
workbook.
Compiled into the binary rather than read from disk. A browser has no directory to
read, and fetching eight files before rendering anything would be absurd.
Overrides come in as text from whoever can obtain them: step-cli reads a
directory, a SPA takes an upload.
Behind a new election_config_templates feature, separate from `reports` — that one
also brings a headless browser and three AWS SDKs, and this needs only the
template engine.
The interesting decision is escaping. Handlebars escapes for HTML by default,
which is the wrong language here: a quote would become ", valid JSON holding
the wrong text. Turning escaping off instead would let a stray quote break the
document. So the escape function is a JSON string escape, which is stricter than
what the Python did and means a custom template interpolating something
undisciplined still renders parseable JSON. The builtin templates only
interpolate ids, timestamps and enum values and never need it; client text is
deep-merged into the parsed result, never rendered.
A helper's output bypasses the engine's escape function. {{json}} relies on that —
the point is to emit a JSON literal, and a quoted object would not be one — while
{{default}} has to escape for itself, because it lands inside a string. Both are
tested, since the difference is invisible until a fallback contains a quote.
An override for a name nobody renders is refused rather than ignored: elections.hbs
in a templates directory is a typo, and silently rendering the builtin would leave
its author staring at output that ignores their edit.
A template that renders invalid JSON quotes the lines around the failure with the
offending one marked. A bare parse error against a hundred lines of rendered
output is not debuggable.
Strict mode is off on purpose: a template referring to a field this entity has no
value for should leave it blank for the merge to fill, not stop the build.
Related: sequentech/meta#12769
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Rows to entities: each one a rendered template with the row's dotted-path columns deep-merged over it, identified by a uuid5, and joined to the others by external_id. Nothing reaches for a clock or a filesystem, so the same build runs in step-cli and in a browser and produces the same bytes. what_it_builds_is_a_bundle_the_platform_accepts is the test that makes sharing this worth anything: the document the builder produces deserializes into ImportElectionEventSchema — the importer's own struct — and passes validate(), the importer's own rules. Two implementations that merely looked similar would not. Problems accumulate rather than stopping at the first one, and each names the sheet and row it came from. An author fixing a spreadsheet wants the whole list, not one round trip per mistake, and a bundle path is no use to whoever has to edit the file. A test with three separate mistakes in it asserts all three are reported. The decisions carried over, each now with a test that says why: A control column is consumed, not merged. election.external_id is how a contest names its election, not a field called external_id on an object called election. Areas are read in two passes so a parent may appear below its own child; authors do not sort their spreadsheets topologically. An area may not be its own parent. An area needs a name, and two areas may not share one, because the voters CSV resolves a voter's area by name. A duplicate silently assigns voters to whichever one the importer finds first. A base export is merged under the templates, never over them, and its identity fields are scrubbed first: its ids, its bulletin board, its keys, and a statistics and status block describing a run that already happened. Carrying any of those over produces an event that looks configured and is not. Identity is reasserted after the merge, so adding a field cannot reintroduce the bug. reports stays an empty array and scheduled_events stays null. Both travel in their own CSV; a populated array here is silently dropped, which is how a report goes missing without an error. A parameter nothing interprets is recorded in election_event.annotations with a warning rather than dropped, because dropping it is how a setting goes missing on election day. A parameter with no value is a placeholder the author left blank, and says so. An id typed as a number matches the same number written as text. Whether a cell was formatted as a number is not something an author controls per column. One message improved rather than ported: the Python's "a election reference is required" is now phrased without an article to get wrong. This level builds the JSON document. The CSV members, the auth presets and the Keycloak realm are the next levels; keycloak_event_realm is null until then. Related: sequentech/meta#12769 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four of a bundle's parts are not in the JSON document at all. Voters and scheduled events are always CSVs, reports are a CSV or nothing, and admin users, role permissions and communication templates are tenant- or portal-scoped rather than part of an event import. All of them resolve external_ids against the entities the previous level built, which is why they live beside it — build_tables.rs is a child module of build.rs so the two share the resolved ids while each file stays readable. The scheduled-events table is where the voting window actually lives: scheduled_events in the JSON document is not read by the importer at all. Its rows go out through emit's JsonField, so a SQL NULL is a bare null and the payload is a CSV-quoted JSON literal, and the task id is built by the same function that mirrors generate_manage_date_task_name — a different shape means a task that never fires. What the tests pin down: A voter's area travels as a name, because that is what the importer resolves by. A voter with no authorized-election-ids is authorized for all of them; writing an empty attribute would deny access to every election instead. A voter with an email address is treated as verified, because an unverified address blocks delivery of the one-time code and a census address is one the client asserts is correct. Any column the builder does not derive is carried through as a Keycloak user attribute, which is how a client adds a reporting breakout column with no code change. A passthrough column blank for every voter is dropped, and that is not cosmetic: get_copy_from_query treats the mere presence of a password header as "hash a password for each of these voters", so a blank one would give every voter an empty credential. A column name outside ^[a-zA-Z0-9._-]+$ is refused here, because both CSV importers reject it mid-import with nothing naming the column. An author may write an event type the way they say it — "start voting period" and "start-voting-period" both reach START_VOTING_PERIOD. An election whose window never opens or never closes is warned about: it imports fine and then quietly never opens, and an event-wide row covers every election. A report's encryption_policy and permission_label are read from the row, not from the rendered template. They are control columns, so the row was excluded from the merge and the template value is only a default — reading it instead is how configured_password silently became unencrypted once. The permission matrix is transposed: a matrix is what a human can check at a glance, and role,permissions is what export_tenant_config.rs writes. A template document gets its newlines back, because literal \n and \" are what survive a copy-paste out of a JSON export. Unescaped in one pass, so an escaped backslash before an n does not become a line break. One dead branch removed rather than ported. The Python warns when a census has no email or mobile column at all, but `email` is a derived column and is always present, so that branch could never fire. A census with no contact column reaches the per-voter count with every voter unreachable, which is the more useful message anyway; a test says so. Related: sequentech/meta#12769 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The last of janitor's Python: the four authentication presets, the realm settings an event already implies, and the permission-label check that explains an empty Elections list. Presets are patches, not realms. A realm is ~165 kB of interdependent Keycloak configuration whose client URLs belong to the environment it was exported from, and the importer takes keycloak_event_realm wholesale — a present realm replaces the environment's provisioned default rather than merging into it. So a preset is applied to a realm someone exported from a working event, and is always also kept on its own so nothing the document asked for is silently dropped. With no base export, no realm is emitted and the patch says so out loud. Two things a deep merge cannot express are now struct fields rather than magic keys inside the patch, which is how the Python carried them: binding an authenticator config, and patching the user profile. That removes the strip-the-directives step the writer used to need, and with it the chance of forgetting it. The realm work worth reading: identityProviders and authenticatorConfig are merged by alias, not replaced. They are referenced by alias from elsewhere in the realm, so replacing either wholesale would strip providers the environment configured on purpose. The realm name is derived from tenant and event, because the voting portal and the smart-link URLs derive it the same way — it is not a free choice. A base export's own event id is swapped out of its client URLs first, because import remaps every UUID it finds and a stale one would be remapped to something unrelated. The user profile is parsed, patched and re-serialised rather than merged: it lives inside a Keycloak component as a single JSON string. A preset naming a flow or authenticator the target realm lacks is warned about rather than applied blindly, which is only possible because each preset states what it needs and why. The event's own languages, title and login CSS become realm settings, because the platform carries none of them across: it never syncs supportedLocales, and has no path at all from an event name to a realm display name. Language codes go through the platform's own iso_639_2t_to_bcp47 rather than a second copy of it — the Python transcribed all 177 entries by hand. Basque and Dutch are missing from that table and pass through unconverted; both implementations always behaved that way, so it is a gap in util::locale rather than a regression, and a test says so. CSS is escaped for java.text.MessageFormat, quotes before braces so the quotes this adds are not themselves doubled, and written for every enabled locale — Keycloak looks the message up in the voter's language, so CSS under `en` alone vanishes when a voter switches to Spanish. A stylesheet copied out of a working realm arrives already quoted and is unwrapped first. uses_otp now gates the voter-reachability warning. Under SAML or digital certificates the client's identity provider authenticates the voter, so "56 of 56 voters cannot be sent a one-time code" is noise rather than a finding. The permission-label check is the one that matters most in practice. An election whose label no administrator holds imports cleanly, reports no error, and then does not appear in the Elections list at all — it happened on the first real import, where a document labelled an election dlc-officers-dburs while its own administrators carried dlc-officers. Warnings rather than errors, because administrators may already exist in the target tenant carrying labels this file knows nothing about. Related: sequentech/meta#12769 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The last piece before a CLI: what a built bundle is written as, and the zip the Admin Portal accepts. Pure, like the rest of the module — named byte blobs, no filesystem. step-cli writes them to a directory, a browser offers them as downloads, and neither leaves a half-written output behind because nothing is written until everything is built. The line worth reading is between the two groups. Importable members go inside the zip. Administrators, roles and communication templates go beside it: they are tenant- or portal-scoped, and putting them in the zip would mean importing an election event could silently create administrator accounts. A test asserts admin_users.csv is in one list and not the other. The scheduled-events member is written even when empty. The voting window lives in it, so whether the file exists must not depend on whether the source had a sheet. The reports member is the opposite: absent when there are none, because an empty reports CSV is not a valid one. The realm patch is written whether or not it was applied, and its comment says which — the two need opposite things done next. Because the bind-authenticator and user-profile directives are struct fields rather than keys inside the patch, there is nothing to strip here, and the file can instead state each one explicitly with a note that it is not a merge. Whoever applies it by hand cannot deduce that from the patch itself. The zip is reproducible: a fixed timestamp and a fixed mode on every entry. Without them the archive's bytes change on every run and "regenerating produced no diff" stops being something anyone can check. A test zips the same bundle twice and compares. Behind a new election_config_archive feature, and the layout half is behind the builder's. A front end that only validates an existing bundle has nothing to write and should carry no zip writer. One deliberate difference from the Python: every JSON file is two-space indented rather than one-space for the event document and two for the rest. Cosmetic, since the importer parses it, but it does mean the first regeneration of an existing event reindents that file once. Related: sequentech/meta#12769 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
step-cli step build-election-event -w workbook.xlsx -o out Every decision it makes lives in sequent_core::election_config — the same module windmill validates with and the browser-side tools will run. The command only talks to the filesystem and to whoever ran it; reading a workbook, building, validating and laying out the files are pure functions in the core, which is why nothing is written until every one of them has succeeded. It produces the same bundle the Python does. Run against the real SEIU1000 workbook and diffed against janitor's output for the same file with the same tenant id: export_voters, export_scheduled_events, export_reports, export_permissions, admin_users, every template and templates.json are byte-identical; the event document is identical when parsed, differing only in indentation. The event id and the derived tenant id match to the character, which the pinned ids tests predicted. Every warning matches too, including the dlc-officers-dburs permission label that made every election invisible on the first real import. That diff caught a real bug, which is the point of having two implementations to compare. admin_users.csv did not match: an international phone number arrived as 33645312453 instead of +33645312453. The workbook computes contact details with a formula, so the cell is t="str" with a cached string result, and calamine 0.26 tried a float parse on that string first — Rust's float parser accepts a leading plus — so the number silently lost its country prefix and Utils.sendCode would have texted a number with no country code. Fixed by moving to calamine 0.36, where t="str" returns a string unconditionally. Two tests pin it: one for the leading plus, one for a formula result that is text but looks numeric. Validation runs twice on purpose, because they are different questions. The builder reports what is wrong with the workbook, in sheet-and-row terms an author can act on. validate() then reports what would be wrong with the bundle — the same check windmill runs before importing. --check-only reports without writing. --strict refuses to write when there are warnings, which is what CI wants: a warning means the bundle imports and the configuration probably is not what its author meant. --base-export reads a .json or an export .zip. --templates-dir overrides any of the eight entity templates and says which it took; a .hbs file whose name is not one of them is reported rather than ignored, because that is a typo and rendering the builtin would leave its author staring at output that ignores their edit. --auth-preset none builds without configuring authentication, which the SEIU workbook needs: it declares SAML and leaves the IdP metadata URL blank pending the client's identity provider. Unread sheets are named, so a misspelled tab does not silently drop its entities. The output distinguishes what to import from what travels beside it, and says outright that admin_users.csv is a secret. Also: sheet-level problems no longer claim "row 0", which names no row a spreadsheet has and reads as a bug. Origin::sheet and Origin::column say what they mean. Related: sequentech/meta#12769 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
--out rather than --out-dir, and --created-at, both as the Python janitor spelled them. The documentation for those is already written and already read; renaming them for no reason costs someone a lookup. --check-only keeps its name rather than the Python's --validate-only, because it is the same thing the importer calls check_only on the server, and the two being one word apart is more confusing than either name alone. Related: sequentech/meta#12769 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
election_config::wasm is a thin wrapper and nothing more: the work happens in the same functions step-cli and windmill call, and this converts between them and JavaScript. That is the point — a file that validates in a browser imports on the server, because the same code decided both times. checkBundle ships in the existing WASM package today, with no workflow change: election_config::wasm is gated the way crate::wasm is, and the build already enables both features it needs. So the admin portal can tell an operator what is wrong with an export before they upload it, and the answer is the importer's own. buildFromWorkbook and authPresets are additionally gated on the xlsx and archive features, which that build does not enable — so admin-portal, voting-portal, ui-core and ballot-verifier gain the validator and carry no spreadsheet parser, template engine or zip writer. The packaging for a build that does enable them lands with the SPA that needs it, rather than being guessed at here. The finding worth reading: the zip crate's default features pull bzip2, zstd and lzma, which are C libraries with no wasm32 target. Left as they were, nothing in this module could ever have compiled for a browser — the whole point of the work — and the failure would have surfaced much later, in a front end's build. Now default-features = false with deflate only, which is the only method an import archive uses. calamine already restricted its own zip the same way. Returns plain JS values rather than wasm-bindgen classes. A front end holds these in state, hands them to React and serialises them; an opaque handle with a free() method is a memory leak waiting for whoever forgets to call it. A failed build returns its problems rather than throwing. A list of problems is something a page can render; an exception is not. The one exception is a file that does not parse as an export at all, where there is no list to render. The TypeScript interface for Problem, Report and BuildOutput is declared here rather than written again in the front end, and authPresets returns the presets rather than a dropdown duplicating them — so a UI cannot offer a preset that does not exist or miss one that does. Verified: type-checks with the CI feature set, tests and clippy unchanged. The wasm32 target itself could not be built on this machine — ring's build script needs a clang that targets wasm32, which Apple's does not have — so the cross-compile was not exercised locally; CI builds that target today. Related: sequentech/meta#12769 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The exporter wrote both files itself. Now it writes them through election_config::emit, the same code step-cli and the browser use, so an export and a generated bundle are one shape rather than two that happen to agree. The scheduled-events writer is the one worth reading. It derived both the header and each row from serde_json::to_value(event).as_object(), taking .keys() and .values(). That produces the right file today, but only because three unstated things line up: the preserve_order feature is enabled somewhere in the dependency graph, so a serde_json::Map iterates in insertion order rather than alphabetically; insertion order is ScheduledEvent's field order; and that order happens to match what the importer reads. import_scheduled_events.rs takes the payload from record.get(10). Sorted alphabetically, index 10 is task_id and the payload is at 5 — so every exported event would import with its payload read as a task name, and the task that opens voting would never fire. Reordering the struct, a refactor with no visible risk, or losing preserve_order from the graph would have done that silently, on the disaster recovery path. The row is now built field by field against SCHEDULED_EVENT_COLUMNS. The order is the code rather than a consequence of it. The reports CSV wrote "ID", "Election ID", … as its header. Harmless, because process_reports_file skips the header and reads by index, but it meant an export and a generated bundle differed on sight for no reason. REPORT_COLUMNS is now the one name for that file's shape, and the permission-label separator comes from MULTI_VALUE_SEPARATOR rather than a second literal "|". One byte-level change: emit writes \n line endings where csv::Writer wrote \r\n. The reader accepts either, and it makes an export diffable against a generated bundle. cargo test -p windmill --lib: 235 passed, 2 ignored, unchanged. Related: sequentech/meta#12769 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The point of one validator is that two callers reach the same answer. Tests written separately in Rust and in TypeScript would not prove that — each would prove only that it agrees with itself. So the cases are data: compiled in here, handed to the browser through the WASM surface, and both sides run the same list. A case is a patch, not a whole bundle. sound.json is the one bundle anybody has to keep valid; each case says what it changes and what that should be worth. Reading a case means reading the difference, which is what a case is about — and it means there is one place to fix when the schema gains a required field, not eight. The verdict is the distinct codes, not counts. A case is about what kind of thing is wrong; how many times validation says so should be free to change without a fixture needing an edit. Eight cases, each carrying the reason it exists: sound, a contest naming no election, two elections sharing an id, areas in a parent loop, more winners than candidates, a preferential ballot counted by plurality, nothing on any ballot, and an election hidden by a permission label — the one that cost a real import. Three tests guard the suite rather than the code. One asserts at least eight cases loaded, because a loop over an empty list passes while checking nothing. One asserts the base bundle is itself sound, so a problem in it is not attributed to whichever case is read first. One asserts every code validation can produce has a case, and names the three that deliberately do not: MissingField and InvalidValue come from a bundle that fails to deserialize at all or from reading a source document, and MissingSchedule needs scheduled events, which arrive with the builder. Not a substitute for validate_tests.rs, which pins exact messages and paths. These pin the verdict, which is the part two languages have to agree on. Related: sequentech/meta#12769 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fixtureCases() returns the same list the Rust tests run, as data rather than something a front end reimplements. A page asserting checkBundle(case.bundle) matches case.expect is checking that the browser and the server reach the same verdict — which is the only thing that makes one validator worth having. A suite written separately in TypeScript would prove only that each side agrees with itself. Related: sequentech/meta#12769 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A second package from the same crate, with election_config_xlsx, _templates and _archive turned on. The four front ends that already vendor sequent-core build without those on purpose. They get the bundle schema and the validator; a spreadsheet parser, a template engine and a zip writer have no business in the voting portal. Turning the features on in that build would have been one line and the wrong line. wasm-pack takes the npm package name from the crate name, so both builds would otherwise produce sequent-core-0.1.0.tgz. The script renames the manifest between build and pack — with node rather than sed, because package.json is JSON and a regex over it is how a build script starts corrupting files that contain the same string twice. Built in CI even though nothing vendors the result yet, because this is the only place that compiles those features for wasm32, and that is the part most likely to break. zip's default features pull bzip2, zstd and lzma, none of which have a wasm32 target; restricting it to deflate is what makes the build possible at all, and a CI step is what keeps it that way when someone adds the next dependency. The package is uploaded as an artifact so it can be vendored without a local nix toolchain. Related: sequentech/meta#12769 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…oduces The wizard is not a different kind of election event. It is a different way of filling in the same fields — so a Blueprint does not become a bundle here, it becomes a Workbook, and the existing builder takes it from there. That is the whole design, and it is why this module is as small as it is. Going through the workbook shape means the architect inherits the entity templates, the deterministic ids, the CSV byte shapes, the Keycloak realm handling, the archive layout and every validation rule, with no second copy of any of them. What is left is only what is genuinely the architect's: its plan, the checks that apply to a plan rather than a bundle, and the three files it produces that are not part of an import. a_plan_becomes_a_bundle_the_platform_accepts is the test that says it worked: the compiled export deserializes into ImportElectionEventSchema and passes validate(). What the TypeScript got wrong, each now a test: Its output was not the importable format — election_config.json inside a nested official_election_setup.zip, where the importer looks for export_election_event-<uuid>.json at the archive root. It stamped new Date() into every entity, so no two runs of the same answers agreed. It embedded a Keycloak realm copied from one environment. The importer takes keycloak_event_realm wholesale, so that would have replaced whatever the target environment had provisioned. Nothing is invented now; a realm comes from a base export or not at all. It hard-coded winning_candidates_num to 1 while letting max_votes be anything, so a "choose 3" contest quietly elected one person. It validated nothing. The plan is now a saved artifact rather than something reconstructed by parsing the generated bundle back in. That round trip lost every answer the bundle has no field for — the trustee threshold, the ceremony dates, the points of contact — and broke whenever the bundle's shape changed. blueprint.json travels with the output and reopens exactly, and every optional field has a default so an older plan still opens. validate_plan asks questions in the wizard's own vocabulary, which validate() cannot: a threshold no number of trustees can meet, which is the worst failure there is — everything works until the tally and then nothing can be decrypted; a key ceremony after voting opens, when the key has to exist before a vote can be encrypted with it; a tally before voting closes. Both passes run, because they are different questions. One decision worth naming: the wizard does not do districting, so one area is synthesised covering everybody, and every contest lands on it. Without an area and a ballot link there is no ballot at all. It is named "All voters" so that a delivery engineer opening the event can see nobody chose it. Related: sequentech/meta#12769 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Areas, with parents, and per-contest assignment. A union with locals inside regions is the ordinary case, not an advanced one, and a wizard that cannot express it sends somebody to the spreadsheet for the one thing they came here to do. An area's name is plain text rather than translated, and that is not an oversight: the voters CSV identifies a voter's area by name, so it is an identifier the importer matches on. Two areas sharing one would put voters in whichever the importer found first, so that is an error. Assignment is explicit rather than inherited down the tree. A contest on a parent is not automatically on its children's ballots, so "who votes on this" is answerable by reading one list instead of walking a tree — which is the question somebody actually asks, usually the day before voting opens. An empty assignment means every area. A contest nobody has got to yet should not silently fall off every ballot, and a plan with one area always wants it. A plan with no areas at all still gets the single synthesised one, so districting stays optional and an older plan reads exactly as it did. Assigning the same area twice produces one link, because both rows would mint the same id and one would overwrite the other. Thirteen tests, including that a districted plan still compiles to a bundle the platform accepts, that the parent tree survives, that a local contest reaches only the ballots it names, and that a plan saved before any of this existed still opens. Related: sequentech/meta#12769 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`pub mod build` is behind `election_config_templates`; `pub use build::{…}`
was not. `default_features` does not imply that feature, so `sequent-core`
did not compile for the feature sets windmill, harvest, velvet, b4, orare and
the browser builds actually declare — `error[E0432]: unresolved import build`.
`cargo build --workspace` hid it by unifying step-cli's features across the
graph, so it only ever failed in CI, where each crate is checked on its own.
Fourteen jobs across two PRs were red on this one line.
Also here: `architect`'s re-export carried the same `#[cfg]` twice, and
`presets`' re-export was gated although `pub mod presets` is not — the inverse
mistake, harmless but wrong, and it kept `AuthPreset` and `RealmPatch` out of
builds entitled to them.
Verified at every gate — none, `default_features`, `election_config_templates`,
`election_config_archive`, `election_config_xlsx` — and `cargo check -p windmill`,
which is the one that reproduces the original failure.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>JSON carries no comments, so the two fixture files could not hold an SPDX header and `reuse lint` counted them missing — which is what `.license` sidecars are for. `reuse lint`: 3184 / 3184, compliant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The ungated re-export fixed in the previous commit was invisible locally — `cargo build --workspace` unifies features across the graph, so the gate that was broken is one nobody builds by hand. It surfaced as fourteen red jobs in the slow suites, each blaming itself. Six `cargo check --lib` runs, one per gate consumers actually ask for. Two minutes, and the failing job is named for the gate rather than for whichever crate noticed. `--lib` rather than `--all-targets` on purpose: the test modules need `default_features`, and whether they compile is what the test jobs are for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A plan said `2027-03-01T09:00`. That went verbatim into the ScheduledEvents
sheet, verbatim into `cron_config.scheduled_date`, and then met this:
// windmill/src/tasks/scheduled_events.rs
ISO8601::to_date(&scheduled_date).ok() // parse_from_rfc3339
RFC 3339 requires an offset. A bare wall clock does not parse, so `get_datetime`
returns `None`, the poller's filter drops the event, and **voting never opens** —
on the day, with nothing anywhere on that path reporting anything. The fixtures
used exactly that shape, so the tests agreed with the bug.
The workbook path was safe only by accident: `Cell::DateTime` goes out through
`Utc.from_utc_datetime(naive).to_rfc3339()`, which does carry one.
So a moment is now three things — the wall clock somebody typed, the IANA zone
they typed it in, and the offset that turns it into an instant:
Timestamp { local: "2027-03-01T09:00",
zone: "America/Los_Angeles",
offset_minutes: -480 } → 2027-03-01T09:00:00-08:00
**No timezone database.** This module compiles to wasm32, and `chrono-tz` is
about a megabyte of tables to answer something the browser already knows:
`new Date(local).getTimezoneOffset()` gives the right offset for that date,
daylight saving included. Whoever picks the time resolves the offset; this side
records it, checks it and computes with it.
Keeping the typed text matters too — a plan reopened next month should read back
as its author wrote it, not converted into wherever the reader happens to be.
Ordering now compares instants. As text, Tokyo's 09:00 sorts after Los Angeles'
08:00; as instants it is seventeen hours earlier, and the old comparison said
the wrong thing for any plan whose moments were not all in one zone.
Validation gained: a `local` that is not a date and time; an offset outside
±14:00, or in range but not a multiple of fifteen minutes, which is what an
offset out by minutes rather than hours looks like; a warning when no zone is
named, because a schedule handed to a client without one is how two people
arrive an hour apart; and a warning when the voting window crosses a clock
change, which is legitimate and invisible in the clock times.
A plan written before any of this still opens: a bare string reads as UTC, which
is what it always meant.
`the_scheduled_date_parses_the_way_the_platform_parses_it` asserts with
`DateTime::parse_from_rfc3339` — the platform's own call, not a lookalike — and
`a_bare_wall_clock_is_what_the_scheduler_cannot_read` pins the reason this module
exists.
396 passed, 0 failed. fmt and clippy clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>`to_workbook` and `side_files` had no callers outside their own tests. There was no `compile_plan` anywhere, and `wasm.rs` exported nothing that took a `Blueprint` — so the Rust half could validate a plan and map it to rows, the React half could collect one, and nothing joined them. Both halves were green: 388 Rust tests, 82 TypeScript tests, and every React test injecting a fake core. `compile_plan` is the join, and it is six steps of sequencing rather than a second builder: validate_plan → to_workbook → build → validate → layout → side_files The fourth is the one worth having. The built export is deserialized into `ImportElectionEventSchema` — the importer's own struct — and run through `validate()`, the importer's own rules. That is the same second pass `step-cli` and `buildFromWorkbook` each make, and it belongs here rather than in every caller, because "a plan that compiles produces a bundle the platform accepts" is the property this module exists to guarantee. `side_files` output now joins `layout.auxiliary`, which is already the field meaning "must not go inside the archive". A ceremony schedule inside the importable zip would suggest it was part of the import. `validatePlan` and `compilePlan` are gated on `election_config_archive` alone, not also on `election_config_xlsx`: compiling a plan needs the templates, the builder and the zip writer, but no spreadsheet parser, so the wizard's package does not carry calamine. The options reader both entry points share is lifted out of `build_from_workbook` for the same reason the resolved struct and its patch share a macro — two copies drift the first time `BuildOptions` grows. 400 passed, 0 failed. fmt and clippy clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
With the ungated re-export fixed, `build_wasm` compiled for wasm32 for the first time — the package built and `wasm-opt` ran — and then fell over on the last line: Error: Unable to find the pkg directory at path "pkg-election-config", or in a child directory of "pkg-election-config" `wasm-pack pack` takes the **crate** directory and looks for a `pkg` child inside it. That is why `wasm-pack pack .` works for the default build, whose output goes to `./pkg`. Given a custom `--out-dir` it goes hunting for `pkg-election-config/pkg`, which is not a thing. `wasm-pack pack` is a wrapper around `npm pack` in the output directory, so that is what this does now, and it leaves the tarball exactly where the artifact upload expects it. The wasm32 build itself is unaffected and was already succeeding — including `validatePlan` and `compilePlan`, whose first compile for that target this was. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A profile is how one customer's Election Architect differs from another's: values they never choose, screens they never see, fields they must fill in. ## Paths, not top-level keys The TypeScript version could lock any top-level key of its config and nothing deeper — `LockableConfigKey = Exclude<keyof ElectionConfig, 'elections'>`, with ballot structure carved out because it has to stay editable. That is not enough for what clients ask. `clients/smart-td.json` locks `defaultCountingAlgorithm`, but what SMART TD wants is "every contest is plurality-at-large" — and locking the event-wide default leaves every per-contest override open, which defeats the lock entirely. So a profile speaks in paths, with `[]` for every element of a list: elections[].contests[].overrides.tally.counting_algorithm One entry, every contest, however many there are. The range is deliberately tiny: literal segments and `[]`, nothing else. No globs, because nobody can predict what one does. No indices, because `elections[2]` breaks the moment somebody reorders their ballot — refused with that sentence as the message. A path naming a field no plan has is refused rather than ignored, because a profile with a typo in it configures nothing, silently, and nobody finds out until a client asks why their build looks like everybody else's. ## Defaults seed; locks force Two different things. A locked or hidden path is written unconditionally, which is what makes the lock hold against a hand-edited plan. Any other default is written only where the plan says nothing, so it seeds a new plan without discarding an answer somebody gave. Zero and `false` count as answers. Treating them as empty is how a default quietly overwrites a deliberate choice — there is a test for the threshold case. ## One enforcement point The TypeScript guarded this twice, in `stripLockedUpdates` on write and `reapplyLockedFields` on import, and both were bypassable by editing the saved JSON. Here `apply_profile` runs as step one of `compile_plan`, before validation — so the locked value is the one that gets checked and the one that gets built. `a_locked_value_reaches_the_built_bundle` asserts it end to end, because a lock that only holds in the plan is decoration. `hidden` is drawing, not permission. The module says so at the top, and `readProfile` hands the front end the paths rather than a verdict: Rust decides which paths, and which screens that empties is a question about screens. ## Required fields are problems, not a boolean They go onto the report with the path that owns them, so the wizard's existing router puts each on the step that can fix it. The TypeScript needed `FIELD_WIZARD_STEP` — thirty keys mapped to steps by hand — and `isFieldFilled`, a thirty-arm switch. Both existed only because its paths were flat, and both are gone. ## Also here `Profile` carries its own warnings rather than dropping them, which a test caught: locking a path with no default fixes it at whatever the plan happens to say, and that was being computed and thrown away. 424 passed, 0 failed. fmt clean; clippy reports nothing in these modules. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…olds `validate_plan` and `to_workbook` take a `&Sources`, and `check_census` and `voters_sheet` read the census from it a batch at a time rather than iterating a `Vec` the plan carries. Behaviour is unchanged: `Sources::from_plan` derives the source from the fields the plan still has, so every existing test asks the same question of the same rows and gets the same answer. That is also the danger. A change whose whole point is that nothing changes will pass just as well if the checks go on reading `plan.voters`, so `the_census_comes_from_the_source` hands over a plan whose census is empty and a source that has one, and asks for the two answers only a census can give — a Voters sheet, and a problem about a voter's area. Sabotaged both ways before it was believed. Three things came out of the rewrite rather than being planned: - `check_census` made two passes over the census, the second only to ask whether any voter named an area at all. One pass answers both. - Four call sites reported an unreadable census under one name, which `every_named_problem_has_its_own_name` refuses and is right to. They are one message, so there is now one function that makes it. - The per-row half of the no-area report had no test — only the aggregate did — so the pass that merged them could have dropped it silently. `unreadable_census` is new because a source can fail where a `Vec` could not: a file that vanished between the picker and the build, a sheet whose header moved. The columns of the Voters sheet now come from `CensusSource::columns()` instead of from a scan of every row, which is the shape `build_realm::census_attributes` needs when it moves in the next commit — it declares a Keycloak user-profile attribute per census column, and a source that could only answer by reading ten million rows would make that a non-answer. `Sources::from_plan` is scaffolding and says so. It goes away in the same change as the fields. Refs: meta#12769
… row
**A non-object user profile component aborted the build.** `components[…][0]` can hold
any JSON value, and `.expect("a realm component is an object")` turned a malformed
base export into a panic instead of a message. Reported through `self.warn` now, the
same way the missing-attributes case two blocks above already is.
**`offset_minutes * 60` could overflow.** It is `i32` arithmetic, `offset_minutes`
deserializes from any `i32`, and `instant()` runs before `check()` does — so a value
above `i32::MAX / 60` panicked in debug and wrapped in release, producing a wrong
instant rather than a problem. `checked_mul` folds it into the existing "not a usable
offset" problem.
**Two schedule rows for one processor and one election silently became one.** Both the
uuid5 and the task id derive from the processor and the election alone, so the second
row emitted the same identity, the importer kept one, and the other scheduled time
was lost with no message. Rejected as a `DuplicateId` naming the earlier row, which is
what `require_unique` does for voters.
Two tests: the overflow reports rather than panics, and the duplicate schedule is
refused. 575 sequent-core tests, fmt clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>**A whitespace-only `authorized-election-ids` denied every election.** An absent cell
means "all of them" and takes an early return; a cell holding only spaces is
*present*, so every entry fell to the `is_empty` guard, `resolved` stayed empty, and
the voter was written an empty attribute — which denies access to everything, with
nothing reported. A blank cell now means what an absent one means.
**`row.text` read ids differently from the rest of the builder.** It answers only for
a string cell and does not trim, while `require_external_id` and `resolve` read the
same columns through `value_as_text` and trim — so a numeric or space-padded
`external_id` registered in the first Areas pass and vanished in the second. Worse in
`area_contests`, where the two reads key the duplicate check: two different numeric
pairs both keyed as `("", "")`. Both reads go through `value_as_text` now.
**Plan validation only caught an area inside itself.** A two-hop loop — A inside B
inside A — passed `validate_plan` and surfaced later from the bundle validator, in
generated-id vocabulary the author never wrote. `climbs_into_a_loop` walks the parent
chain with a visited set and stops at the first repeat.
One test for the loop; the other two are covered by the existing build suites.
576 sequent-core tests, fmt clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>**A realm that would not re-read became a JSON string.** Swapping the base event's id through the encoded realm can fail to parse, and the fallback returned `Value::String(swapped)` — so `keycloak_event_realm` held text where the importer expects an object, and the importer took it wholesale. It now carries the realm over unchanged and says so, which is the lesser fault. **The fixture coverage check could not fail.** Its comment claimed that adding a `Code` without a case fails the test; it walked a hard-coded list, so a new variant was neither covered nor reported. Every variant is now walked and placed by an exhaustive `match`, so a new one does not compile until somebody decides which side it belongs on — which is how `ConflictingColumns`, absent from the old list, got placed. **The numeric-id test covered Elections only**, and the Areas and AreaContests builders read their reference cells through a different accessor — the defect the previous commit fixed. Extended to both sheets, asserting two areas and two links rather than a collapsed pair. **Two stale comments:** `names()` returns declaration order rather than sorted, and `scheduled_event.hbs` pointed at `build.py` for values this port computes in `election_config::build_tables`. 577 sequent-core tests, fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two lists that had to agree with the platform and were written out again beside it — the same fault as the counting algorithms in #2981, and the same fix. **`EVENT_PROCESSORS` is gone.** `event_processor` parses through `EventProcessors::from_str` and returns the typed value, so the eleven strings are the enum's `strum` spellings and nothing else. The fuzzy normalisation authors rely on — "start voting period", "start-voting-period" — happens before the parse, as it did. `check_voting_windows` compares variants instead of string literals. **`scheduled_event_task_id` is gone too.** Its own doc said it "mirrors `generate_manage_date_task_name`… reproduced rather than approximated", and the template beside it warns that a different shape means a task that never fires. It calls the platform's function now. The two tests that asserted the exact task name still assert it, so they pin the scheduler's shape rather than the copy's. `EventProcessors` gains `EnumIter`, which is what lets the "expected one of" message name the variants instead of a list beside them. 577 sequent-core tests, fmt clean, and the emitted CSV is unchanged — the task-name assertions are byte-for-byte what they were. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n' into HEAD # Conflicts: # packages/sequent-core/src/election_config/time.rs # packages/sequent-core/src/election_config/time_tests.rs
**A misspelled requirement kind checked the wrong collection.** `Requirement.kind` was a `&'static str` and the realm builder matched it with `"flow" => flows, "authenticator_config" => configs, _ => authenticators` — so a typo fell into the authenticator arm, compared against the wrong list and reported nothing. It is a `RequirementKind` now, the match is exhaustive, and the test that asserted the string was one of three is gone because the type says it. **The `feature-gates` job declared no `permissions`**, so it inherited whatever the repository default is, to check out code and run `cargo check`. `contents: read`, and `persist-credentials: false` so the job token does not sit in `.git/config` for every later step to read. 577 sequent-core tests, fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`build` takes a `&Sources`, and `census_attributes` reads `CensusSource::columns()` where a build is handed one. This is the place the census's *shape* decides something outside the census: every column becomes a Keycloak user-profile attribute, and Keycloak drops an attribute its profile does not declare — so a column that fails to reach this list is a value the sign-in flow reads as absent, with no error anywhere. It is the one consequence of moving the census that has no symptom, which is why it gets a test rather than a comment. `the_realm_declares_the_same_attributes_either_way` builds the same census twice, once as the sheet and once as a source, and compares what the realm declares. Sabotage-checked by truncating the source's columns. **`build_voters` deliberately still reads the Voters sheet**, and the reason is worth having written down. A source yields `PlannedVoter`, which cannot carry `enabled`, `email_verified` or `authorized-election-ids`: all three are in `RowShape::OWNED`, so they are excluded from `extra`, and there is no field for them. Routing the builder through a source today would switch on a voter the census switched off and authorise a restricted voter for every election in the event — silently, in the delivery. `a_source_cannot_yet_say_what_the_voters_sheet_says` pins the gap in both directions: nothing through the shape, all three through the sheet. Nor would it save anything yet. The Voters tab stays in the workbook, so the census is already in memory at build time whatever `build_voters` reads; iterating a source instead would add a walk and move no bytes. Also fixes a feature combination `EA-F4-054` broke: `sources` was left ungated on the theory that a plan describes these things whether or not the build can render a template. It cannot — `validate_plan` lives in `architect`, which is behind `election_config_templates` — so `--features election_config_xlsx` alone would not compile. All eight combinations check clean now. The wasm boundary does not move: `buildFromWorkbook` passes an empty `Sources`, so the JS side is unchanged and beyond's CI keeps building green against this tip. Refs: meta#12769
Nine commits landed on this branch while EA-F4-056 was being written. Two of them do not merge as text. `Requirement::kind` became a typed `RequirementKind` — because the `&str` it replaced fell through a `_ =>` arm meaning "authenticator", so a misspelled kind was checked against the wrong collection of the realm and reported nothing. But `NeedsDoc::kind` is a *profile's* text, read at run time, and is exactly where a misspelling comes from. So `as_requirement` returns `Option<Requirement>` and refuses a kind it does not recognise, rather than reintroducing the guess one layer up. `a_requirement_kind_nobody_recognises_is_not_guessed_at` covers the three that parse, whitespace around them, and the ones that used to be treated silently as authenticators. `areas_inside_each_other_are_refused_by_the_plan_validator` arrived written against both the older `PlannedArea` and the older `validate_plan`; it goes through the module's `checked` helper like every other test here. 924 tests, every feature combination clean, `cargo fmt --check` clean. Refs: meta#12769
The wasm boundary grows two optional fields and loses nothing. `compilePlan` and `previewBallot` accept `options.census` — a `CensusPull`, which is three methods rather than an array — and `options.files`, the bytes the plan's file names refer to. A host that passes neither behaves exactly as it did: the census and the files are read off the plan. That is the whole design constraint here. beyond's CI checks out step's live tip and builds the WASM in the same job, so a boundary that only grows is the only kind that can be pushed before the browser half exists. This is the gate the beyond commits sit behind, and it can sit on the branch indefinitely without breaking anything. `CensusPull` is an extern type, not a deserialised one, and the reason is the point of the exercise: ten million members reaching Rust as one JS array is precisely what `serde_wasm_bindgen::from_value` would do with them. Its shape is `CensusCsvReader`'s own, which is not a coincidence — that class already exists and the wizard's census store already speaks to it, so a CSV the browser has parsed once is not parsed again to be handed over. **A `CensusCsvReader` is a `CensusPull`.** `JsCensus` reads the columns once, at construction, and keeps them. `CensusSource::columns` hands back a borrowed slice so it cannot call into JavaScript — and should not, since `census_attributes` asks per build. Reading them up front is also what makes the promise true: the column list is available before the first row. `compile_plan` takes a `Compile` struct rather than a fifth positional argument. Rust would have caught a misordered call, but five arguments is where the next `compile_plan_js` mistake hides — a profile passed as a third positional that the browser never sent, so every profiled build compiled with `None` and no locked value reached a bundle, silently, on both sides. Two things that had to move with it, or the new option would be decorative: - `plan_images` and `plan_materials` prefer a named file from `sources.files` over the plan's own bytes. Which way round matters — a plan opened from a save file carries names and no bytes; one the wizard is holding carries both. - `check_logo` refused a logo that names a file and carries nothing. That is the exact shape of a reopened plan, so it now asks the sources too. Without this the first thing the new option does is fail validation. `the_caller_may_bring_the_census_and_the_files` is the Rust half of the browser feature and the only half testable without one: a plan carrying no voters and no logo bytes, beside sources carrying both, looked for in the finished bundle. 925 tests, every feature gate clean, `cargo fmt --check` clean, and the wasm feature builds. Refs: meta#12769
`archive::save_file` writes `<external_id>-plan.zip` — `blueprint.json`, `census.csv`, `files/<name>`. The wizard stops handing over a bare `blueprint.json`, because a plan on its own is no longer the whole document: it is a document with the members' names and the candidates' photographs missing, which is the kind of loss nobody notices until the file is reopened somewhere else. A bare `blueprint.json` still *opens*, and always will — somebody has one saved from last month, and refusing it would be refusing their work. `open_delivery` reads a save file and a delivery with one function, because they are one layout twice: both carry `blueprint.json` at the root, and what differs is where the bulk sits. The root is looked at first, the importable zip second. **The delivery branch used to read `blueprint.json` and stop.** It returned `Report::default()` and never looked inside `official_election_setup.zip`, so reopening a delivery gave back whatever the JSON still happened to carry. That is fine today and is nothing at all the moment a plan stops carrying its own bulk — a defect made invisible by the very duplication this programme removes. `a_delivery_brings_its_census_and_its_files_back` is the test; it fails against the old behaviour. `Opened` gains `sources`, derived from the plan for every door with nothing else to offer and read from the archive for the two that have. A caller hands it straight to `compile_plan` without asking what kind of file it opened. Two smaller things the round trip forced: - `sources::cell_of` is the inverse of `RowShape::voter`, written beside it so the writer is not a fourth copy of *what a census row means*. - `plan_file_name` turns `images/document_<uuid>_<name>` back into the `<name>` a plan points at, splitting exactly twice — a photograph called `photo_of_ada.jpg` keeps every underscore it came with, and a greedy split would invent a file name nothing references. The comma is in the round-trip test on purpose. `O'Brien, Jr.` is an ordinary member's name and an unquoted CSV field turns them into two members. 928 tests, every feature gate clean, wasm builds, `cargo fmt --check` clean. Refs: meta#12769
Three new exports beside the old ones, and nothing removed. beyond's CI builds this crate's live tip, so additive is the only thing that can be pushed first; `openConfiguration` and `planInDelivery` stay until the browser half has moved. `CensusHandle` is `CensusPull`'s three methods pointing the other way. A host that opens a save file gets one and can hand it straight back to `compilePlan` as `options.census` — so ten million members are read out of the zip, checked, counted and written into a bundle without ever being a JavaScript value. `CensusCsvReader` is the same shape and stays, because a dropped CSV has no `Opened` to come from. That the two are interchangeable is the reason the interface was written before either of them. `takeCensus()` takes rather than borrows. A handle owns its cursor and two of them over one census would each believe they were at the start; the second call returns `undefined` instead of a second reader of the same rows. `saveFile` returns the zip. `files()` serialises in one go, unlike the census, because a logo and some photographs are values a host puts in state rather than a stream it pulls. Refs: meta#12769
…rt declares `check_sources` asks three questions of a plan and its files, and the first has no other home. **Two owners, one file name.** Bytes are keyed by name from the moment they leave the plan — `sources.files`, a save file's `files/` directory, `BuildOptions::images` — so two candidates whose photographs are both `photo.jpg` are not two photographs. One silently becomes the other's, and the wrong face is on a ballot. Nothing catches this today, because a plan carrying its bytes inline holds two different values under two identical names. The moment it stops, they are one, and the check has to exist before the field goes rather than after. **A file the plan names and nobody holds** is an error, said against the field somebody filled in. The build already says this — `material.file-missing`, `logo.file-missing` — at build time, in the workbook's vocabulary, about a spreadsheet column the author may never have seen. **A file nobody names** is a warning: dead weight in a delivery rather than a broken one, and usually a photograph whose candidate was renamed afterwards. And the cross-check, which is a different kind of thing and deliberately narrow. When an election-event archive carries both a realm and a voters CSV, the census's columns are compared with the user-profile attributes that realm declares. A column Keycloak has never heard of reaches nobody — the platform drops an attribute its profile does not declare, with no error and nothing in a log — so an export whose own realm does not declare its own census's columns is worth one sentence, said where somebody can still ask for a better export. Two artifacts that arrived together, compared where both are in hand. **Nothing is stored about what a census ought to contain**, so there is nothing to keep in step and nothing to be wrong. It is also asserted the other way: the check stays quiet on the wizard's own output, because `declare_census_attributes` puts every census column into the profile, and a warning that fired on every export this tool writes would be worth nothing. `sources::OWNED` is public now — it is the list of columns the platform owns, and the cross-check needs the same one the reader uses rather than a fourth copy. 931 tests. Refs: meta#12769
`read_plan` documented itself as "the only way a plan should be deserialized" and
had **no callers outside the tests**. `open`, `compilePlan`, `previewBallot`,
`applyProfile` and `step-cli` each reached for `serde_json` or
`serde_wasm_bindgen` directly, so `migrate_v1` and `migrate_v2` ran nowhere but in
a unit test.
That is a shipped defect, not a tidiness point. A version 2 plan keys its voters
by area *name* and the builder reads `area.external_id`; opened unmigrated, the
name lands in the identifier field and every voter's area dangles — in a plan that
opened without a word of complaint. `an_older_plan_is_migrated_on_the_way_in` is
the test.
`read_plan_value` is the funnel for callers who already have the document parsed:
the wasm boundary hands over a `JsValue` and `step-cli` a file it has read, and
neither should have to serialise back to text to be read properly. Both return
`ReadPlan { plan, sources }` — the sources derived from the plan's own fields
today, and lifted out of the JSON by the migration when those fields go. That is
the reason this is a commit of its own and comes before the removal: a caller
still holding its own `serde_json::from_str` would hand back a plan whose census
is silently empty, against a green suite.
`nothing_else_deserializes_a_plan` reads the sources rather than behaviour,
because the failure mode is a *new* call site and nothing at run time can notice a
migration that was skipped.
932 tests.
Refs: meta#12769Telling a save file from a delivery by "does it carry a `census.csv`" looked equivalent to telling them apart by the nested zip, and is not. A plan with no members writes an archive whose single member is `blueprint.json`, and that was read as a delivery — so the wizard's own save file was the one file it could not recognise, and only for the plans most likely to be saved early. Found by beyond's core contract check on the first plan it tried, which is the argument for that file existing. Refs: meta#12769
The wizard validates on every keystroke, and this is where a census that is no longer inside the plan has to arrive — otherwise duplicate usernames and dangling areas stop being reported while somebody is still in a position to fix them. Optional, like the other two, and a caller that passes nothing still means "read them off the plan". Refs: meta#12769
They reached one file and it was the wrong shape of place. `admin_portal/communication_templates.json` is a *loose* member of the delivery — `archive::admin_portal_member` does not match that name — so it sits outside `admin_portal_settings.zip` while the CSV of the same concept sits inside it. Two files, two places, one idea, and whoever loads them has to know that. `compile_plan` appends each message to `bundle.templates`, which is the list `export_templates-<tenant>.csv` and `templates/*.hbs` are both written from. The seam is deliberate: the *builder* still mints nothing from a message — `messages_leave_as_two_files_outside_the_bundle` still holds — and the wizard's compile adds them afterwards. **Not a Templates sheet, and the first attempt was.** A sheet has to be merged with the one a janitor's workbook carries, and the merged row then has to survive `a_plan_round_trips_to_the_same_workbook`: the same cells, in the same order, coerced the same way, with every `Translated` filled per language exactly as the Messages sheet fills it. It failed on all three, and for no gain — the delivery's spreadsheet already has a Messages tab, so the sheet would have said the same thing twice. Deleted rather than debugged. This does not put a template in the election-event import. `templates/` and `export_templates-` are both `admin_portal_member`, so they travel in the Portal's settings zip, which is what a tenant's templates are loaded through and what that invariant is really about. Asserted both ways. A template the workbook already carries under the same alias wins: two with one alias is what `build_templates` refuses outright, and a client's own wording is not a screen's to replace. **The loose JSON stays**, as asked — "also add". It is now a second copy of content that has a home, which is the duplication this programme removes everywhere else; retiring it is in the open list. 936 tests. Refs: meta#12769
Version 4. `plan.voters` is gone, and a plan describing ten million people is no longer ten million rows of JSON — saved, re-serialised on every compile, and copied across the wasm boundary each time, while the same rows also went into the workbook and into `export_voters-<id>.csv`. `migrate_v3` is the one migration that hands something back. `migrate_v1` and `migrate_v2` rewrite a document in place and that is the whole of their job; this one *removes* a field, and the rows in it are somebody's members. Dropping them would open a saved plan as an election with nobody in it — silently, against a file that still looks complete. So it lifts them out, `read_plan_value` puts them in `ReadPlan::sources`, and the caller hands that to `compile_plan` exactly as it would a census read from a file. It also **removes** the key, so nothing downstream can find a second copy, and it keys on version 3 so a stray `voters` in a current document is not silently adopted as a census the plan does not believe it has. Both are tested, as is the client's own column surviving the lift. The two other doors that carried a census now hand one back beside the plan: `plan_from_workbook::ReadPlan` gains `sources`, and `fill_from_archive` returns them. `voters_into` became `voters_from_csv`, which returns rows rather than writing them into a plan that has nowhere to put them. `voters` joins the control paths in `profile.rs`. A delivery profile hides the Census screen, and there is no `voters` in the shape of a plan any more — the honest field path would name a row of a file the plan does not contain. Refused in `defaults` like the rest, because a starting value for a census is not a thing. `Sources::from_plan` keeps only its files half. The three `bytes` fields stay for now and the reason is the sequencing rule: the browser reads a logo and a candidate's photograph off the plan, so removing them here would lose every photograph on reopening until beyond holds those beside the plan too. That is the next pair, and it is a much smaller one — a logo is kilobytes. Also fixes a feature gate `EA-F4-067` broke: `add_message_templates` carried `election_config_archive` while its caller is behind `election_config_templates`, so that combination alone would not compile. Every combination checks clean now — and checking only the three I had changed is how it got through. 938 tests, wasm builds, `cargo fmt --check` clean. Refs: meta#12769
The bottom of this stack — "One definition of the import bundle" — is on main now, as a squash. That is why six files conflicted with no real disagreement in them: main has `problem`, `report`, `schema` and `validate` from that squash, and this branch has the same content plus everything the four commits above it added, so ours is a strict superset. Checked rather than assumed: `git diff ded5be6..origin/main` over `election_config` and `types/ceremonies.rs` is **empty**, so nothing on main has touched those files since the squash and there is nothing of theirs to lose. Taking ours for all six. 948 tests (main brings ten), step-cli green, the wasm feature builds, `cargo fmt --check` clean. Refs: meta#12769
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
A fast-forward would have left this branch pointing at a commit that already belongs to `architect-fields`, and GitHub attributes one check suite per commit — so the milestone pull request inherited #2988's suite and ran nothing of its own. `build_wasm` in particular never ran for this branch name, which is what beyond's end-to-end job resolves through `.github/step-branch` to fetch the WebAssembly it tests against. A merge commit gives the branch a commit of its own with the same tree, so the milestone is built and tested as itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…on ids EA-F4-077. Two defects an import found, both in what the importable zip says about voters. **An empty census is now no census.** `layout` wrote `export_voters-<id>.csv` unconditionally, and with no Voters sheet `build_voters` returns a table with no columns *and* no rows — so the member came out as a single newline: no header, no data. The platform's importer still reads it as a census and refuses the whole import, which made an election whose membership list has not arrived yet impossible to import at all. Now written only when there are voters, the same rule `reports` already followed. **A voter who names no election gets no election ids.** `voter_elections` expanded a blank or absent `authorized-election-ids` to *every* election in the event, on the reasoning that an empty attribute would deny access to all of them. Importing it says otherwise: the area already carries the voter's ballot, and an `AreaContests` row is what puts it in front of them. So the expansion wrote a restriction the census never expressed — which is what a client saw as an election id appearing against every voter they had not restricted — and it goes stale the moment an election is added. Blank now stays blank; a voter who names elections still has them resolved to ids, and naming one that does not exist is still refused. Two comments that claimed the old behaviour are corrected rather than left to mislead: `voters_sheet`'s "`authorized-election-ids` from the areas", and `build_voters`'s note on what a source cannot yet say. 950 tests pass and all seven election-config feature combinations compile. Verified in the wizard against a freshly built wasm: the sample builds a zip whose census member is present with an empty elections column, and the same plan with its two voters removed builds one with no census member at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
EA-F4-079. `locked` requires a matching `defaults` key, for a good reason: a
lock with nothing to lock to fixes the field at whatever the plan happens to say.
There is one class of path where no value could ever be right, and it made
*fixed* unreachable for the two settings on the Areas screen a delivery profile
most wants to take away.
`defaults` holds one value per path and `apply_profile` writes it to every
element the path resolves to. One identifier shared by every area is a duplicate
by construction — `check_unique_identifiers` refuses the build it makes — and an
area's identifier is derived from its name anyway, one per area. So *fixed* here
means what a delivery engineer means by it: the client does not get to type one.
`parent_external_id` is the same read the other way round: locking **Inside**
says a client's districting is flat, and no value expresses that better than the
absence of one.
So `derives_its_own_value` names those two paths, exempts them from needing a
default, and — the half that matters more — **refuses them one**. `is_fixed`
writes a default unconditionally, so `{"areas[].external_id": ""}` would blank
every area's identifier on every compile and report "an area needs an
identifier" about a box the client cannot see. That is EA-F4-052 one rung
deeper, and this closes the door before anybody walks through it.
Additive: the rule accepts a profile it used to refuse, and refuses only a
profile nobody could have wanted. 953 tests pass, all feature combinations
compile, fmt and clippy clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Two commits from `architect-fields`. Kept as a merge commit rather than a fast-forward for the reason the last one was: GitHub attributes one check suite per commit, so a tip shared with `architect-fields` leaves this branch's pull request inheriting #2988's suite and running nothing of its own — including `build_wasm`, which beyond's end-to-end job resolves through `.github/step-branch` to fetch the WebAssembly it tests against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
EA-F4-081. Reported as: download a delivery whose voters have areas, drop the zip back on Getting Started, and the voters arrive with no area. `RowShape::voter` and `cell_of` are meant to be each other's inverse — the comment above them says so, "the two together are the whole answer to what a census row means". They were not. `voter` has always read `area_name`, because that is the column the platform's own export writes; `cell_of` had no arm for it and fell through to `extra`, which is empty for a column a `PlannedVoter` owns. So a census whose header says `area_name` came out of `next_batch` with that cell blank. The CSV inside a delivery says exactly that, so reopening a delivery handed the wizard a census whose every area was empty — while the plan's own areas came back intact, which is what made it look like a wizard bug. The identifier is what goes back, not the display name: the identifier is what the voter holds and what `voter()` put there. `build_tables::voter_area_name` is still the one place that translates back, at the boundary that writes the platform's CSV — the only reader that wants a name. Pinned by a round-trip property test over three headers rather than by a case for `area_name`: read a row through a shape, write it back out, get the row. It fails with the old `cell_of` on the delivery's own header, and that is the check that would have caught this when the shape was introduced. 955 tests pass, every feature combination compiles, fmt and clippy clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
One commit: a census written back keeps its area. Kept as a merge commit rather than a fast-forward, because GitHub attributes one check suite per commit — a tip shared with `architect-fields` leaves this branch's pull request inheriting #2988's suite and running nothing of its own, `build_wasm` included, which is what beyond's end-to-end job resolves through `.github/step-branch`. Co-Authored-By: Claude Opus 5 <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 freeto 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.
Parent issue: https://github.com/sequentech/meta/issues/12769
v0.0.1 of the Election Architect — step's half, as one diff.
This is not a new change. It is the tip of the step stack targeting
main, so the whole milestone can be read, linked and previewed in one place:Review the layers there — each shows its own work against the layer below. Read this one for what v0.0.1 is.
Why a branch rather than a tag
A tag names a commit; it does not give you a diff against
main, a place to comment, or a preview to click. Those four pull requests each answer "what did this layer add", and none of them answers "what are we shipping".The preview
beyond's companion is sequentech/beyond#834, whose
.github/step-branchnames this branch — so the wizard in that preview is built from this step, not from the stack tip. The WebAssembly core and the vendored voter's ballot both come from here.https://docs-internal.sequentech.io/architect/pr-preview/pr-834/
That preview is the artefact worth opening: the shared core in this PR is what the browser runs.
What this half contains
sequent-core::election_config— the bundle schema, the report types, and one validator the server and the browser both runstep-cli step build-election-eventandcompile-plan, plus the browser bindings and the shared fixture casesui-essentials, so a preview renders the portal's own components rather than a copyDraft, and why
It stays a draft until the stack lands. Merging this instead of the stack would land four reviews as one commit.