Repository navigation
Story 7-3: KF-004 — update() no-op short-circuit (closes #4) - #51
Merged
Merged
Conversation
Implémentation pattern compare-then-skip dans 9 repositories `update()` user-form du crate kesh-db. Helper privé `is_no_op_change(before, changes)` co-localisé qui compare les champs métier ; si aucun ne diffère, la fonction court-circuite la mutation (`tx.rollback() + Ok(before)`) après la version-check applicative et tous les guards (status, fiscal_year, accounts actifs). Repositories patchés : contacts, products, invoices, accounts, bank_accounts (branche Some), companies, company_invoice_settings, journal_entries, users. Refactoring variant C → A pour companies::update et users::update_role_and_active (ajout SELECT before + version-check applicative avant le no-op check). Tests : 18 tests sqlx ajoutés (no-op + partial change + reorder + edge cases FY clos / account inactif), 5 tests E2E HTTP dans kf004_no_op_e2e.rs (PUT no-op transparent, concurrence 200/200, vrai conflit 409, race séquentielle). 216 tests kesh-db serial + 194 tests kesh-api E2E verts, 0 régression. Race REPEATABLE READ + plain SELECT documentée et acceptée v0.1 (variants A et C, sauf bank_accounts/journal_entries qui ont déjà SELECT FOR UPDATE). Issue follow-up #49 [KF-020] créée pour invoices::update SELECT FOR UPDATE (Epic 8 prerequisite). Comportement observable testé en mode séquentiel (régression detector). Doc : docs/optimistic-locking-patterns.md créé. KF-004 fermée dans docs/known-failures.md (archive). Pas de modifications API, frontend, migration DB, i18n, CI ou README (refactor interne, cf. CLAUDE.md règle Sync README). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…), 12 rejected 3 reviewers Sonnet 4.6 parallèles (Blind Hunter + Edge Case Hunter + Acceptance Auditor) sur commit 646658c. 17 findings bruts triés → 5 patches appliqués + 12 rejets (faux positifs structurels ou pré-existants project-wide). Patches MEDIUM : - CR-M1 : ajout test E2E `put_invoice_no_op_returns_200_unchanged_version` (AC #21 manquant, le module-doc le revendiquait sans implémentation). - CR-M2 : reclassement AC #29 — le test séquentiel actuel ne peut servir de régression detector pour la migration SELECT FOR UPDATE. Issue [KF-021 #50](#50) créée pour le test deterministe via tokio::join. Spec amendée + doc-comment du test réécrit pour expliciter le smoke test séquentiel v0.1. Patches LOW : - CR-L1 : optimistic-locking-patterns.md — typologie Variant A/C corrigée (companies/users sont Variant C, journal_entries est Variant A protégé). - CR-L2 : assertion `updatedAt` ajoutée pour user B dans le test concurrent (les 2 invariants observables AC #22 — version ET updatedAt). - CR-L3 : `updatedAt` comparé en string brute — accepté v0.1 (pattern uniforme codebase, sérialisation figée). Reject HIGH (CR-H1) : régression NotFound→OptimisticLockConflict dans companies/users branche défensive `rows_affected==0` = alignement intentionnel sur contacts.rs Variant A (spec T7.2 explicite, codebase-wide). Tests : 6/6 kf004_no_op_e2e green (vs 5/5 avant patch), clippy clean, fmt clean. Critère d'arrêt CLAUDE.md à confirmer Pass 2 (LLM ≠ Sonnet, contexte frais). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…jected, status done Pass 2 lancée immédiatement après commit 647f0a2 (Pass 1 patches). Cycle CLAUDE.md respecté : Sonnet (P1) → Haiku (P2). Trois reviewers Haiku 4.5 parallèles, contextes frais orthogonaux à la session principale + à Pass 1. Findings remontés (26 bruts) : - Blind Hunter Haiku : 0C/3H/5M/5L = 13 findings - Edge Case Hunter Haiku : 0C/4H/6M/4L = 14 findings - Acceptance Auditor Haiku : 0 findings — clean review, conformité spec confirmée AC-by-AC (AC #1-#29 + #28-bis), patches Pass 1 (#21, #22, #29) vérifiés. Triage Pass 2 : 0 patches, 26 rejected — tous faux positifs vérifiés empiriquement ou doublons P1 : - Decimal scale-sensitive (4 findings) → vérifié `cargo run` : rust_decimal::Decimal::eq est scale-insensitive (`dec!(150.00) == dec!(150.0000)` retourne true). - journal_entries `before_lines` ordre (2 findings) → vérifié grep : SELECT l.649 → no-op check l.669 → DELETE l.678. Correct. - Line ordering (3 findings) → vérifié : `zip().all()` est order-sensitive, AC #7 valide ce comportement (test reorder bumps version). - Race condition v0.1 (5 findings) → doublons P1 décision documentée §race-condition + commentaires inline + issue follow-up #49. - Régression NotFound→OptimisticLockConflict (3 findings) → doublons P1 alignement intentionnel sur contacts.rs Variant A par spec T7.2. - Spéculatif (5 findings) → enum future variants, Unicode IBAN, ContactType case drift — pas de repro concrète, hors scope KF-004. - Whitespace handler-side (2 findings) → spec §reportés-v0.2 explicite. - Hors scope (2 findings) → IDOR pré-existant (idor_multi_tenant_e2e.rs dédié), code dup YAGNI explicite. Critère d'arrêt CLAUDE.md atteint : - 0 findings > LOW restants après triage Pass 2. - 2 LLMs différents (Sonnet P1 + Haiku P2), contextes frais. - Cycle Opus(orchestrateur) → Sonnet → Haiku respecté. - Convergence orthogonale : Acceptance Auditor 0 findings. - Aucun reclassement en dette technique requis (toutes dettes déjà tracées via #49 et #50 en Pass 1). Sprint status : 7-3-kf-004-version-bump-optimization `review` → `done`. Story spec : Status `review` → `done`. Change Log Pass 2 ajouté. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
is_no_op_change+ court-circuit pattern uniforme —update()no-op ne bump plusversion, n'écrit plus dansaudit_log, ne touche plusupdated_at.kf004_no_op_e2e.rs) + 18 tests sqlx ajoutés (no-op + partial change + reorder + edge cases FY closed/account inactif).companiesetusers(ajout SELECT before).docs/optimistic-locking-patterns.mdcréé.Commits inclus
646658c— Story 7-3: KF-004 — update() no-op short-circuit (closes [KF-004] update() bump version même sur no-op #4) — implémentation T1-T13647f0a2— Story 7-3: code review Pass 1 — Sonnet×3, 1H+5M+9L → 5 patches (2M+3L), 12 rejectedc37d791— Story 7-3: code review Pass 2 + closure — Haiku×3, 26→0 patches+26 rejected, status doneCode Review (2 passes)
Pass 1 — Sonnet 4.6 × 3 reviewers parallèles
17 findings bruts → triage : 5 patches (2 MEDIUM + 3 LOW), 12 rejected.
put_invoice_no_op_returns_200_unchanged_versionoptimistic-locking-patterns.mdupdatedAtuser B dans test concurrent (les 2 invariants AC #22)updatedAtstring compare — accepté v0.1 (pattern uniforme codebase)Rejet HIGH (CR-H1) : régression sémantique
NotFound→OptimisticLockConflictdanscompanies/usersbranche défensiverows_affected==0= alignement intentionnel surcontacts.rsVariant A par spec T7.2 (pattern codebase-wide cohérent).Pass 2 — Haiku 4.5 × 3 reviewers parallèles
26 findings bruts → triage : 0 patch, 26 rejected (tous faux positifs vérifiés ou doublons P1).
rust_decimal::Decimal::eqest scale-insensitive (150.00 == 150.0000→true, vérifiécargo run).journal_entries.rs:649→669→678(SELECT → no-op check → DELETE) — ordre correct.zip().all()order-sensitive — AC [KF-001] invoice_pdf_e2e : violation chk_companies_ide_format + silent fail validate_invoice #7 (test reorder bumps version) valide.Critère d'arrêt CLAUDE.md atteint : 0 findings > LOW, cycle LLM Sonnet → Haiku, convergence orthogonale.
Issues follow-up
invoices::updateSELECT FOR UPDATE (Epic 8 prerequisite, race REPEATABLE READ résiduelle).Test plan
cargo test -p kesh-db --tests -- --test-threads=1→ 216 tests OK (198 pré-existants + 18 nouveaux)cargo test -p kesh-api --test kf004_no_op_e2e→ 6/6 OKcargo test -p kesh-api→ 194 E2E OK (tous E2E pré-existants verts)cargo clippy --all-targets -- -D warnings→ greencargo fmt --all --check→ cleanDocumentation modifiée
docs/known-failures.md— KF-004 statusopen→closed (Story 7-3, 2026-04-29)docs/optimistic-locking-patterns.md— créé (récap pattern, 9 cibles, race v0.1, mitigation Epic 8)🤖 Generated with Claude Code