From 6932816c4218e6838e5a5f9d5b6cc04288790b9f Mon Sep 17 00:00:00 2001 From: Bendik Aagaard Lynghaug Date: Wed, 12 Aug 2026 22:55:36 +0200 Subject: [PATCH] Lighten the comment load Drops porting-history narratives (dodrenett), superseded-behavior explanations, and restatements of what the next line does. Constraint notes (fail-closed policies, CAS semantics, cascade behavior, id uniqueness) stay, just shorter. No code changes. Co-Authored-By: Claude Sonnet 5 --- src/aggregates/mod.rs | 6 +- src/answers.rs | 78 ++++++++--------------- src/app.rs | 18 ++---- src/chain.rs | 11 +--- src/content.rs | 141 ++++++++++++++---------------------------- src/events/mod.rs | 9 +-- src/events/store.rs | 56 +++++------------ src/upload.rs | 8 +-- 8 files changed, 105 insertions(+), 222 deletions(-) diff --git a/src/aggregates/mod.rs b/src/aggregates/mod.rs index 2e3d56c..9688071 100644 --- a/src/aggregates/mod.rs +++ b/src/aggregates/mod.rs @@ -174,10 +174,8 @@ pub enum TransitionError { /// No creation event found for this id - nothing to transition. UnknownAggregate, DisallowedTransition { from: String, to: String }, - /// A concurrent writer already advanced this aggregate past the - /// sequence this call read - the fix for the lost-update race - /// `answers::transition_answer` used to have (see - /// `crate::events::store::append_event`'s CAS). + /// A concurrent writer advanced this aggregate past the sequence + /// this call read (see `events::store::append_event`'s CAS). Conflict, Store(String), } diff --git a/src/answers.rs b/src/answers.rs index eb0938c..ee8800f 100644 --- a/src/answers.rs +++ b/src/answers.rs @@ -1,16 +1,8 @@ -//! Read-model storage for submitted answers - the KV-backed projection -//! of the durable event log now underneath it (`events::store`, -//! `aggregates`) for every bucket that has a declared state graph in -//! `AppState.aggregates` (loaded from `aggregates.yaml` - see -//! `content::load_aggregates_from_gitea`). An `Alternative.record_as`/ -//! `ResourceSpec`'s bucket is still just a bucket name as far as -//! `resource::get_resource` is concerned - this file is where that name -//! additionally gets checked against the real, content-declared -//! transition table, for the buckets that have one. A bucket with no -//! entry in `aggregates` still works exactly as before (direct KV -//! mutate-in-place, no event log, no CAS) - this is deliberately not a -//! hard cutover, so content isn't forced to declare a state graph -//! before it needs one. +//! Read-model storage for submitted answers: the KV projection of the +//! event log (`events::store`, `aggregates`) for every bucket with a +//! state graph in `aggregates.yaml`. A bucket with no graph still +//! works - direct KV mutate, no event log, no CAS - so content isn't +//! forced to declare one before it needs it. use leptos::prelude::*; use serde::{Deserialize, Serialize}; @@ -95,11 +87,9 @@ pub async fn store_answer( Ok(()) } -/// One selected row's pending transition - `feature_name` is carried -/// per-item (not once per call) so a batch can span more than one -/// resource feature on the same alternative, even though today's -/// content never actually declares more than one transitionable -/// resource per alternative. +/// One selected row's pending transition. `feature_name` is per-item +/// so a batch can span more than one resource feature on the same +/// alternative. #[derive(Clone, Debug, Serialize, Deserialize)] pub struct TransitionItem { pub feature_name: String, @@ -107,15 +97,10 @@ pub struct TransitionItem { pub to: String, } -/// Moves every selected row to its chosen target state in one call - -/// the batched counterpart to what used to be a `transition_answer` -/// fired separately per row, one "Confirm" button each. Now there's -/// one shared button per alternative (see `AlternativeCard`), and a -/// click here applies whatever was selected across every row at once. -/// Each item is independent - there's no cross-item transaction to -/// have, only per-item CAS (see `aggregates::transition`) - so one -/// item's failure doesn't roll back or block the others. Failures are -/// collected and reported together; whatever succeeded stays applied. +/// Moves every selected row to its chosen target state in one call +/// (the shared per-alternative Confirm button). Items are independent +/// - per-item CAS, no cross-item transaction - so one failure doesn't +/// roll back the others; failures are collected and reported together. #[server] pub async fn transition_answers( question_id: String, @@ -233,12 +218,9 @@ async fn apply_transition( .ok_or_else(|| ServerFnError::new("unknown answer"))?; let mut answer: Answer = serde_json::from_slice(&bytes).map_err(|e| ServerFnError::new(e.to_string()))?; - // Matched on (from, to), not to alone - two declared transitions - // may share a target (open -> declined and in_dialogue -> declined), - // and only the one whose `from` is the row's actual current state - // is valid to fire. This also subsumes the old blanket - // "already decided" check: a row in a state no declared transition - // starts from simply has no legal move here. + // Matched on (from, to), not to alone: two declared transitions may + // share a target (open -> declined, in_dialogue -> declined), and + // only the one starting from the row's actual state may fire. let transition = resource .transitions .iter() @@ -255,13 +237,9 @@ async fn apply_transition( let decision_payload_for_event = serde_json::json!({ "to": item.to, "item": item.item_id, "by": user.username }); - // For any bucket with a declared state graph, the real fix for the - // lost-update race two concurrent decisions on the same item used to - // hit: append with CAS on the aggregate's just-replayed sequence, so - // a second racing caller's write is rejected instead of silently - // overwriting the first. A bucket outside `aggregates` falls back to - // the direct KV mutate this always did - not every resource has to - // be event-sourced to keep working. + // CAS on the aggregate's replayed sequence guards two concurrent + // decisions on the same item; a schemaless bucket falls back to the + // plain KV mutate below. if let Some(schema) = state.aggregates.load().get(bucket) { transition_or_reseed( &state.jetstream, @@ -286,10 +264,8 @@ async fn apply_transition( .await .map_err(|e| ServerFnError::new(e.to_string()))?; - // Extend the DAG: the decision is a child node of the answer's own - // submission hash, published the same way any other answer is - - // question_id/alternative come from this call's own arguments and - // the content-declared transition label, never a hardcoded value. + // The decision extends the DAG as a child of the answer's own + // submission hash, published like any other answer. let parent_hashes = vec![item.item_id.clone()]; let chain_hash = hash_node(question_id, &parent_hashes, &decision_payload_for_event, decided_ms); let event = AnswerSubmitted { @@ -307,14 +283,12 @@ async fn apply_transition( Ok(()) } -/// The self-service counterpart to `transition_answer`: no signed-in -/// session, no group check - authorized instead by already holding -/// `item_id` (a chain hash, opaque and unguessable) plus a matching -/// `email`, both carried in the link itself (see -/// `content::SelfTransition`). Deliberately returns the same generic -/// error for "no such item" and "email doesn't match" - a real -/// unsubscribe link should never let someone probe which chain hashes -/// or emails exist. +/// The self-service counterpart to `transition_answers`: no session, +/// no group check - authorized by holding `item_id` (an unguessable +/// chain hash) plus a matching `email`, both carried in the link +/// itself. Deliberately returns the same generic error for "no such +/// item" and "email doesn't match", so an unsubscribe link can't be +/// used to probe which hashes or emails exist. #[server] pub async fn self_transition_answer( question_id: String, diff --git a/src/app.rs b/src/app.rs index 6857cb5..8c2423d 100644 --- a/src/app.rs +++ b/src/app.rs @@ -479,14 +479,9 @@ fn AlternativeCard( } }); - // File fields don't fit a live-typed RwSignal - they get - // their own map of element refs, read (and uploaded) only at submit - // time, and are excluded from `field_map` below. `select` fields - // don't fit it either (a multi-select's value is a *set* of ids, - // not one string) - they get their own `Vec`-signal map, - // plus a plain (non-reactive, content-derived) record of which - // select fields are `multiple`, read back at submit time to decide - // whether to emit a JSON array or a single string. + // Text fields live in field_map; file fields (element refs, read at + // submit) and select fields (a set of ids, plus which are multiple) + // each need their own shape. let mut field_map: std::collections::HashMap> = std::collections::HashMap::new(); let mut file_refs: std::collections::HashMap> = @@ -511,10 +506,9 @@ fn AlternativeCard( } } - // Prefixes field DOM ids with the alternative too, not just the - // question - two alternatives on the same page (e.g. both asking for - // "email") would otherwise mint the same id twice, which is invalid - // HTML and makes