jlevy / jlevy/markform

Two-tier validation model (instant vs deferred) with consistent issue surfacing across CLI and API

Open
#145 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
70
Forks
3
PR merge metrics
No merged PRs in 30d

Description

**Bead:** mf-mhsi
**Plan spec:** `docs/project/specs/active/plan-2026-02-14-tiered-validation-model.md`

## Problem

`markform set` silently accepts semantically invalid values (e.g., lowercase ticker against a pattern constraint), printing only "Form updated" with no indication anything is wrong. The invalid value enters the form and is only caught later by `markform validate`. This surprises both agents and users, who reasonably expect "set succeeded" to mean "value is valid."

More broadly, the spec groups all semantic checks together regardless of cost, and the CLI surfaces issues inconsistently across commands.

## Root cause

The spec (Layer 4) defines two-phase patch validation:

1. **Structural** (pre-apply): field exists, type matches, option ID valid — rejects on failure
2. **Semantic** (post-apply): pattern, range, required, selection counts — accepts value, returns issues

All semantic checks are grouped together and none block writes. But this conflates two very different cost profiles:

- **Fast deterministic checks** (~0ms): pattern regex, min/max, integer, date format, minLength/maxLength, minItems, uniqueItems
- **Expensive checks** (ms-seconds): code validators via jiti, LLM validators (MF/0.2), cross-field validation, external API calls

The fast checks have no reason to be deferred. The expensive checks genuinely must be.

## Proposed changes

### Two-tier validation model (instant vs deferred)

Replace the current two-phase model with two tiers based on cost:

| Tier | Cost | When | Behavior |
|---|---|---|---|
| **Instant** | ~0ms | During `applyPatches()` | Always runs; structural failures reject, constraint failures surface as issues |
| **Deferred** | ms-seconds | During `validate()` / `inspect()` | Must be explicitly triggered |

Within instant validation, there are two failure modes (not separate tiers):
- **Structural failures** (field missing, type wrong) → reject the patch batch
- **Constraint failures** (pattern, min/max, required) → accept the patch, surface issues in `ApplyResult.issues`

### Consistent issue surfacing across CLI and API

**Principle:** Same `InspectIssue[]` data model, channel-appropriate presentation.

Specific changes:
1. **CLI `set`:** Remove `validation_error` filter — surface ALL issue reasons, not just validation errors
2. **`FillResult.remainingIssues`:** Align type with `InspectIssue[]` (currently a subset missing `scope`, `reason`, `blockedBy`)
3. **`InspectIssue`:** Add optional `deferred?: boolean` flag so callers can distinguish instant vs deferred issues

### Open questions

1. Should `set` return a non-zero exit code when issues exist (even though patches were applied)?
2. Is the `FillResult.remainingIssues` type change acceptable as a minor breaking change?

## Implementation phases

1. **Spec + types:** Update spec Layer 4, add `deferred` flag to `InspectIssue`, align `FillResult` type
2. **CLI consistency:** Fix `set` command issue surfacing, add tests
3. **Architecture docs:** Update design docs and QA playbooks

See full plan spec: `docs/project/specs/active/plan-2026-02-14-tiered-validation-model.md`

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.