polls: adopt x-submitMode, and correct nine false statements plus twelve dangling citations - #317
Open
Yaraslaut wants to merge 4 commits into
Open
polls: adopt x-submitMode, and correct nine false statements plus twelve dangling citations#317Yaraslaut wants to merge 4 commits into
Yaraslaut wants to merge 4 commits into
Conversation
`AddComment`, `FinalizePoll` and `UndoLastVoteChange` are the only three `PollModel` actions the shipped `DynamicForm` renders, and all three mutate -- `FinalizePoll` irreversibly. Until now `VoteView.qml` paired each form with `controller: null` and a hand-written Button reaching into `form.previewLine`, because the renderer's default is to fire `submitIfValid` the instant the form is valid: bound to a live controller, that is one comment per keystroke, and one finalized poll on the first optionId digit that named a real option. Explicit-submit mode exists precisely for this and has shipped for a while (`morph::forms::HasExplicitSubmit`, `annotateSubmitMode`, `DynamicForm.qml`'s `explicitSubmitMode` Loader, docs/spec/forms/forms.md's "Explicit submit mode"), but no rung had adopted it -- `grep -rn explicitSubmit examples/` returned nothing. Three hand-built submit widgets in one rung is exactly the shape examples/IMPLEMENTATION.md rule 2 forbids by default, and the rung README justified only the *array* forms, never these. So: declare `static constexpr bool explicitSubmit = true` on the three actions, bind the forms to `page.pollBridge`, and delete the three Buttons. `FinalizePoll`'s extra "not already finalized" gate -- the one thing the schema cannot carry -- moves onto the DynamicForm's own `enabled`, which propagates to the renderer's Submit button through the Frame. `PollBridge::submitIfValid` is now called only from the renderer's own DynamicForm.qml, which lives outside examples/polls/gui/qml and so is invisible to `QmlSurfaceAudit`'s scan. Recorded with `allowUnbound` and a falsifiable reason rather than worked around: the member is still exercised, both by the renderer at run time and directly by this file's own submitIfValid cases. Verified by mutation, not by assertion: dropping `explicitSubmit` from `AddComment` and rebuilding fails both new checks (the parsed-schema case in test_vote_event_dto.cpp and the shipped-document case in test_poll_qml_bridges.cpp). Full suite: 75/75 passed. examples/LADDER.md's forms-gap list still carried explicit-submit as open; it is closed, and now says so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
examples/polls/README.md is required to describe present-tense current behaviour (AGENTS.md). Nine claims in it did not. Each was checked against the tree at origin/master before being rewritten, not taken on faith. Confirmed false, and corrected: - Design decision 5 said a frame refused by `messagesPerSecond` is "dropped silently". The transport answers it with `err "rate limited"` addressed to that frame's own callId (morph#225, src/qt/qt_websocket_server.cpp). The README already said so twice elsewhere -- DD 5 was simply never updated. The same stale claim drove the "the helper's timeout is untested until the limiter actually drops its frames" strain point, which is now stated as done and pointed at the test that proves the answered property. - Design decision 6 claimed there is "no `assignPrimaryAsync` anywhere in the tree", and that `CreatePoll` is a result-keyed creating action whose promote step would abort a WASM page. Both premises are false: `assignPrimaryAsync` exists on `IBackend`, on `QtWebSocketBackend`, and is preferred by `assignHandlerPrimary` itself; and `assignHandlerPrimary` is reachable only under `kShared && ResultKeyed<Action>`, which `CreatePoll` -- registered with no `BRIDGE_KEY_FROM_RESULT`, dispatched through `PollPresenter::_creator`, a plain `NoSharing` handler -- is not. The promote step never runs for this rung at all. The Rallly anchor-UX half of the justification stands and is now the whole of it. - Design decision 7 claimed `morph::model::ModelKey` requires an exact `std::same_as<K, std::string>` or `std::integral<K>` match. It has not since morph#163: `model_key.hpp` defines `WrappedModelKey` explicitly for "the ladder's strong ids", and `lims` keys `SampleModel` on `SampleId` through it. The rung was asserting the framework forbids what another rung does. Restated as this rung's own unmigrated state, with the strong-id migration named as follow-up work (not done here). - "the participant token in `session::Context` is the whole identity" contradicted DD 1 and `PollModel`'s own doc comment 200 lines away: `participantToken` is verified by nothing, and the identity is the action's free-text `participantName` scoped to a pollId that *is* the capability. - "Journal as user feature: vote-change history and undo" contradicted DD 3, which had already resolved undo onto `db::VoteHistoryRecord`; the framework journal is audit-only here. - "`DynamicForm` has no control for a JSON `array` field" is overstated -- the control ships and handles arrays of strings; array-of-*objects* is the real limit, which is the narrower wording poll_schemas.hpp already carried. - "`BridgeHandler::executeJson` silently skips the payload-keyed attach step" was fixed framework-side (morph#68): `registerAction` now builds one executor per `Sharing` policy. Both poll_forms_controller.hpp and poll_schemas.hpp already said so; only the README still called it an open framework gap. What remains is this rung not having migrated `OpenPoll` onto the now-correct generic path. `gui/qml/Main.qml`'s `nativeClient` doc comment repeated DD 6's false framework reason, so it is restated too. The gate itself is **kept**: the Rallly split is a real product decision the rung wanted independently, and removing a shipped client's behaviour on the strength of a docs correction would buy nobody a working organizer path -- this rung still has no native entry point, and the browser client has never been compiled. Verification: `bash scripts/check_spec_citations.sh` passes (581 references scanned, every cited path resolves), and the polls suite is 75/75 green after the Main.qml edit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…st the schema
The previous commit proved the *schema* carries `x-submitMode: "explicit"`,
which is a statement about JSON. What a reader actually wants to know is
whether the three forms now render the shipped renderer's own Submit
button -- and neither the schema test nor the existing smoke cases can see
that: the latter load VoteView with `schemas: {}`, so no DynamicForm has
anything to render.
So load it once more with the real `pollSchemasJson()` document and count
`submitButton` objects. DynamicForm builds that control through a
`Loader { active: form.explicitSubmitMode }`, so a form whose schema lacked
the key has no such object anywhere beneath it -- the count is a direct
observation of the feature, not a proxy for it. `pollBridge` stays null,
so what is counted is what the renderer built, not anything a live
controller caused.
Verified by mutation: removing `explicitSubmit` from `AddComment` and
rebuilding turns the assertion into `2 == 3`. Full suite 76/76 green with
it restored.
Still an engine-load test under examples/TESTING.md presenter rule 6: it
queries the object tree and synthesizes no input events.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s" section `grep -c '^#.*Global Constraints' examples/polls/README.md` is 0, and no polls plan document exists in `docs/superpowers/plans/` either -- yet twelve comments across nine files sent readers to "this rung's Global Constraints", "this plan's Global Constraints", or "the plan's Global Constraints". Every one of them was a dead pointer. Some of the content survived into the README's "Design decisions (resolved before implementation)" section under a numbered decision; some of it lives in a header's own `@file` comment; the plan itself is simply gone. Repointed each site to whatever actually holds its content now, decision by decision rather than with one blanket substitution: - `pollId` being a plain `std::string` (`dto/poll_dto.hpp`, `core/types.hpp`, `db/poll_entity.hpp`) -> resolved design decision 7, which the previous commit rewrote to say this is the rung's own unmigrated state rather than a framework restriction. - `PollEventId`/`PollEventRecord` being a table-wide autoincrement sequence id rather than a timestamp (`core/types.hpp`, `db/poll_entity.hpp`) -> resolved design decision 4. - `CreatePoll` being native-client-only (`gui/qml/Main.qml`, `gui_wasm/main_wasm.cpp`, `gui_lib/poll_qml_bridges.hpp`) -> resolved design decision 6. - The absence of any signed-token mechanism (`gui_wasm/main_wasm.cpp`, whose citation also named a "judgment call 2" the README has never had) -> resolved design decision 1, plus `polls::auth::PollsAuthorizer`'s own `@file` comment. - Tokens being the whole security boundary (`src/models/poll_model.cpp`) -> resolved design decision 1, with the dead plan reference dropped. - The no-relation-typed-member rule (`tests/test_polls_schema.cpp`, twice) -> `db/poll_entity.hpp`'s own `@file` comment, which is where the verified reason actually is: `DataMapper::Update()`'s non-reflection path calls `IsModified()` on every member and no relation type declares it. That was never README content, so pointing at the README would have been a second wrong answer. Two wrong design-decision *numbers* fixed in the same pass, the same defect class in the same rung: - README's Definition of done said the event log needing no epoch token is "exactly as design decision 2 above predicts". That resolution is decision 4; decision 2 is about `authorizeRegister` staying permissive. - `models/poll_model.hpp`'s `GetEventsSince` doc comment made the identical slip, about the identical sentence. Every other `design decision N` citation in the rung was checked against the decision it names and is correct; they are left alone. Comment-only: no behaviour change and no new test. `grep -rni "global constraint" examples/polls/` is now empty, `clang-format --dry-run --Werror` is clean on all eight touched C++ files, `scripts/check_spec_citations.sh` passes (582 references scanned), and the polls suite is 76/76 green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
x-submitMode— the ladder's first real adoptionLADDER.md:260listed "the shipped renderer auto-fires on validity with no submit button" as an open forms gap. It shipped as #258;grep -rn explicitSubmit examples/returned nothing across all six rungs.polls worked around it three times —
VoteView.qml:245,282,317, eachcontroller: nullplus a hand-written Button reaching intoform.previewLine. All three actions are side-effectful andFinalizePollis an irreversible state transition. UnderIMPLEMENTATION.mdrule 2 these are hand-built input widgets, forbidden by default, and the README justified only the array forms.Now
explicitSubmit = trueon the three actions,controller: page.pollBridgebound, three Buttons deleted.FinalizePoll's extra "not already finalized" gate — the one condition a schema cannot carry — moves onto theDynamicForm's ownenabled, which propagates to the renderer's button.Verified by observation, not inference. A new smoke test loads
VoteViewwith the realpollSchemasJson()and countssubmitButtonobjects in the live item tree: 3, withpollBridgestill null. Mutation-checked — droppingexplicitSubmitfrom one action turns it into2 == 3. Two schema tests back it up, including a negative case asserting the six typed-C++-dispatched actions carry nox-submitMode.One audit exemption added, with a falsifiable reason:
submitIfValidis now called by the shipped renderer's ownDynamicForm.qml, which the audit does not scan.Nine false statements in the README — all nine confirmed
Each verified against master before rewriting; none turned out to be wrong about being wrong.
qt_websocket_server.cpp:258sendsmakeErr("rate limited", ...)assignPrimaryAsyncanywhere in the tree"backend.hpp:369,bridge.hpp:817,qt_websocket_backend.cpp:357CreatePollis result-keyed; a WASM tab aborts at promoteassignHandlerPrimaryneedskShared && ResultKeyed;CreatePollis neitherModelKeyrequires exactstd::string/std::integralmodel_key.hpp:62WrappedModelKey; lims already keys onSampleIddb::VoteHistoryRecord; the framework journal is audit-onlyDynamicFormhas no control for a JSONarrayfield"executeJsonskips the payload-keyed attachSharingpolicyOne refinement on the last-but-one: the renderer does not refuse an array-of-objects field, it renders the same control and encodes entries as JSON strings, which the model then rejects. The README now says that.
nativeClientgate: kept, justification restated as purely product-UX. Its framework half is dead twice over, but the rung has nogui/main.cppand its WASM client has never been compiled — dropping the gate would change which never-run binary could theoretically create a poll while widening the diff. The Rallly anchor split is a real product decision independent of any framework property.Main.qml's doc comment was repeating the false framework reason verbatim; that is fixed too.Twelve dangling "Global Constraints" citations
grep -c '^#.*Global Constraints' examples/polls/README.md→ 0. Twelve sites across nine files pointed readers at a heading that has never existed. Found by sweeping forConstraints, which catches sites where the phrase wraps across comment lines and a line-anchored grep misses.Two were not repointed to the README, deliberately:
test_polls_schema.cpp:38,117cite the no-relation-typed-member rule, which the README does not contain — that content lives inpoll_entity.hpp's@filecomment, so they point there. Repointing them at the README would have swapped one wrong answer for another.Four said "this plan's Global Constraints" —
docs/superpowers/plans/holds no polls plan at all. One also cited a "judgment call 2", a second label the README has never had.Applied via a script asserting each pattern occurs exactly once before writing (13/13, no fuzzy matching). Verified empty afterwards:
grep -rni "global constraint\|judgment call\|this plan's" examples/polls/.A wrong cross-reference, twice
README ~429 cited "design decision 2" for the epoch-token resolution, which is decision 4. The same slip appears in
poll_model.hpp:201, about the same sentence — fixed too, since fixing the README's copy and leaving the header's would be odd. All 19design decision Ncitations in the rung were then audited; the other 17 are correct.Verification
ctest -L ladder-polls→ 100% of 76 (baseline 75; the new smoke case is the 76th). Clean under-Weverything -Werror.clang-formatclean.check_spec_citations.sh→ Prose lint OK, 582 references scanned.gui_wasm/main_wasm.cppis Emscripten-only and not built by this configure, so its two comment-only edits have been seen by no compiler here — the same standing gap the README already records for that file.Out of scope, reported
bridge.hpp:64-66cites abridge.mdsection that does not exist;check_spec_citations.shcannot catch it because it checks paths, not section anchors — filed as #316. Thirteencontroller: nullsites remain in bookmarks, lims and pastebin; those files are live in #314/#315 and follow separately.Part of #304 (§B1, §C).
🤖 Generated with Claude Code