Live commit-dialog validation: advisory workflow checks as you type - #758
Merged
Conversation
fiskus
marked this pull request as ready for review
July 10, 2026 11:20
…etches The live-validation LocalResource is read inside the Commit page's Suspense boundary, so each debounced refetch re-armed the boundary's pending set and a plain Suspense unmounted the whole CommitContent subtree (including the JSON editor container) to show its fallback, detaching the editor's DOM and blurring it on every keystroke. Switch the boundary to Transition so it keeps the already-rendered children mounted while later resource loads are pending; the editor now mounts once per dialog open and keeps focus through validation round-trips. Initial load still shows the spinner fallback once.
The commit path validates every field regardless of whether the
metadata parses; the advisory path now does the same, dropping only the
misleading schema check that would have run against {} in place of the
unparseable text.
The namespace doubles as the full package handle that handle_pattern matches; the old name invited swapping in a narrower value.
Member
Author
|
@greptileai Please re-review and update the confidence score. Since your review: the metadata parse error no longer swallows co-present message/handle violations (187f2f8, with a three-violation test), |
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
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
The QuiltSync commit dialog now validates input against the selected workflow's rules live, as the user types (debounced 400ms), surfacing violations before the commit attempt instead of as a commit-time refusal:
handle_patternwhen rules load (the field is read-only in the dialog).Design
load_workflow_rulesfetches and compiles the selected workflow's rules once per (namespace, workflow) into Tauri-managed state, refreshed per dialog open and single-flighted under concurrency;validate_commit_candidateis a pure cache read — zero network I/O on the keystroke path.workflow::validate_candidate_fields+InstalledPackage::workflow_rules; the commit gate'svalidate_packagewas refactored to share the same helpers, behavior-preserving (entries validation deliberately stays with the gate).UserMeta::Keepsemantics will validate at commit.Testing
TDD throughout: gate-parity unit tests in quilt-rs (message/metadata/handle-pattern, entries skip); backend cache tests (fetch-once, per-mount refresh, concurrent single-flight via
tokio::join!, ungoverned no-op, parse-error violation); byte-identical wire-form tests both sides; UI native tests for per-field violation routing, pristine/dirty display gating, debounce key logic, and effective-metadata selection. Full gate green (fmt, clippy native + wasm with denied warnings, 595+ tests, rumdl).Versions
quilt-rs
0.33.0-alpha8, quilt-sync0.18.3-alpha8.Greptile Summary
This PR adds live, advisory workflow validation to the QuiltSync commit dialog: as the user types (debounced 400ms), the message, user-metadata, and package name are checked against the selected workflow's rules, surfacing inline violations per-field before the commit attempt. The commit-time gate is unchanged and remains authoritative; the new checks are purely informational and never block the commit buttons.
WorkflowRulesCache(app-lifetime Tauri state) holds compiled rules keyed by(namespace, workflow_id)inArc<OnceCell<…>>slots that single-flight concurrent fetches;load_workflow_rulespopulates the cache (with a per-session namespace refresh) andvalidate_commit_candidateis a pure cache read with no I/O on the keystroke path. Parse errors on user metadata coexist with message/handle violations rather than early-returning, matching the commit gate's own field-ordering contract.validate_packageis refactored into sharedcheck_field_rules/check_entries_rule/finishhelpers;validate_candidate_fieldsreuses the field helpers while deliberately omitting the entries schema (entries validation stays at commit time).InstalledPackage::workflow_rulesexposes the fetch path for the cache layer.commit.rs): A debouncedLocalResourcedrives the validation round-trip;live_violationsself-keys against the current input to discard stale responses. Message and metadata violations are gated by per-field dirtiness so a pristine form never opens with pre-painted errors; name violations show immediately.Suspenseis replaced byTransitionso the JSON editor is never unmounted during refetches, preventing the focus-loss bug reported after the initial implementation.Confidence Score: 5/5
The change is safe to merge: it is purely additive and advisory — no existing commit gate logic is altered, only reorganised into shared helpers that the new and old paths both call. The previously flagged issues (parse-error early-return swallowing co-present violations, naming ambiguity, JSON editor focus loss on refetch) are all resolved with accompanying tests.
The field-level refactor of validate_package is behavior-preserving: the extracted helpers are called in the same order with the same logic, and a separate test confirms the entries-schema path is correctly excluded from the candidate path. The cache design (Mutex released before await, Arc+OnceCell single-flight, namespace-scoped refresh on dialog open) is sound under Tokio's concurrency model. The UI stale-response self-keying, dirtiness gating, and Transition boundary all address concrete correctness and UX problems identified during review. Test coverage is thorough across cache, validation, wire-form, and UI view-model layers.
No files require special attention.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant User participant CommitPage as commit.rs (UI) participant Debounce as Debounce Effect (400ms) participant Resource as LocalResource (validation) participant Backend as Tauri Backend participant Cache as WorkflowRulesCache participant QuiltRS as quilt-rs (validate_candidate_fields) User->>CommitPage: types in message / metadata field CommitPage->>CommitPage: mark field dirty, update signal CommitPage->>Debounce: live_key changes — arm timer Note over Debounce: cancel previous timer, wait 400ms Debounce->>Resource: debounced_key.set(key) triggers re-run Resource->>Backend: load_workflow_rules(ns, workflow_id, refresh) Backend->>Cache: ensure_loaded() alt cache miss Cache->>QuiltRS: InstalledPackage::workflow_rules() QuiltRS-->>Cache: WorkflowRules (compiled) Cache-->>Backend: rules cached in Arc OnceCell else cache hit Cache-->>Backend: no I/O end Backend-->>Resource: Ok(has_rules) Resource->>Backend: validate_commit_candidate(ns, wf, msg, meta, name) Backend->>Cache: validate() — pure cache read, no I/O Cache->>QuiltRS: validate_candidate_fields(rules, candidate) QuiltRS-->>Cache: Ok / Rejected(violations) Cache-->>Backend: Vec CommitViolation Backend-->>Resource: Vec CommitViolation Resource->>CommitPage: live_violations memo (self-keyed, stale responses discarded) CommitPage->>CommitPage: displayed_violations() — filter by field dirtiness CommitPage->>User: render per-field violation list (advisory, buttons stay enabled)%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant User participant CommitPage as commit.rs (UI) participant Debounce as Debounce Effect (400ms) participant Resource as LocalResource (validation) participant Backend as Tauri Backend participant Cache as WorkflowRulesCache participant QuiltRS as quilt-rs (validate_candidate_fields) User->>CommitPage: types in message / metadata field CommitPage->>CommitPage: mark field dirty, update signal CommitPage->>Debounce: live_key changes — arm timer Note over Debounce: cancel previous timer, wait 400ms Debounce->>Resource: debounced_key.set(key) triggers re-run Resource->>Backend: load_workflow_rules(ns, workflow_id, refresh) Backend->>Cache: ensure_loaded() alt cache miss Cache->>QuiltRS: InstalledPackage::workflow_rules() QuiltRS-->>Cache: WorkflowRules (compiled) Cache-->>Backend: rules cached in Arc OnceCell else cache hit Cache-->>Backend: no I/O end Backend-->>Resource: Ok(has_rules) Resource->>Backend: validate_commit_candidate(ns, wf, msg, meta, name) Backend->>Cache: validate() — pure cache read, no I/O Cache->>QuiltRS: validate_candidate_fields(rules, candidate) QuiltRS-->>Cache: Ok / Rejected(violations) Cache-->>Backend: Vec CommitViolation Backend-->>Resource: Vec CommitViolation Resource->>CommitPage: live_violations memo (self-keyed, stale responses discarded) CommitPage->>CommitPage: displayed_violations() — filter by field dirtiness CommitPage->>User: render per-field violation list (advisory, buttons stay enabled)Reviews (2): Last reviewed commit: "Rename validation_name to validation_han..." | Re-trigger Greptile