diff --git a/.claude/skills/rootline/ref-query.md b/.claude/skills/rootline/ref-query.md index 72a079aa..03851c0a 100644 --- a/.claude/skills/rootline/ref-query.md +++ b/.claude/skills/rootline/ref-query.md @@ -49,6 +49,8 @@ rootline query --sort "prioridad:asc,impact_score:desc" -o json Default (`--output json`) returns structured JSON. With `--select`, use `--output jsonl` or `--output csv` for streaming or processing convenience. `query` is the only command that implements all four formats; elsewhere `jsonl`/`csv` are rejected rather than downgraded to JSON. +**Field-name validation**: an unknown field in `--where` prints `warning: unknown field "estdo" in where expression (did you mean "estado"?)` on stderr and leaves the exit code alone — on `query`, `stats`, `tree`, `graph` and `validate --all`. An unknown field in `--sort` is an **error** (rc=1), like a bad sort direction. Legal names are the query builtins, every key the corpus carries, and every `schema:`/`derive:`/`aggregate:` name the `.stem` chain declares. Treat a zero-result query with no warning as a genuine empty set. + #### JSON (default) Without `--select` (full records): diff --git a/CLAUDE.md b/CLAUDE.md index 31bdfc43..41420ee0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -33,7 +33,7 @@ Pre-commit hooks run `gofmt` + `golangci-lint` + `gitleaks` automatically (`.git - `internal/rules/` — `.stem` file loading, walk-up discovery, top-down merge (parent → child). Discovery collects `.stem` files upward from the target and stops at the first one carrying `root: true` (the declared governance boundary) or, failing that, at the filesystem root — Git is never consulted. A chain that reaches the filesystem root without a `root: true` marker has no declared boundary, and the boundary preflight (`cmd/rootline/preflight.go`) blocks governed commands: on a terminal it offers to add the marker to the proposed project root, and without one it fails with an error. Commands that create schemas (`init`, `schema`) or resolve none (`completion`, `hooks`, `help`, `migrate`) are exempt. Merge is type-driven: maps merge at key level, arrays/scalars replace, null removes. Also contains: validation engine (required, enum, non_empty, exists, requires rules), link schema validation, structural directory rules (require_index, min/max_children), describe output formatting, sequence auto-numbering, validation result types (single + batch), structural integrity via `ValidateStructure` (scoped to the leading frontmatter block through `extract.FrontmatterBounds`; `multiple_yaml_documents` fires only when that block holds more than one YAML document — an unterminated block is reported by the extractor as `malformed_yaml`), v2 match-based field filtering (`match.go`), drift detection between `.stem` and documents (`drift.go`), stem health diagnostics (12 checks: yaml-valid, scope-match, type-consistency, enum-values, rule-field-exists, field-override, aggregated-required, aggregate-formula-coverage, stem-files-exist, monotonic-violations, unknown-check-keys, nested-root-marker — called by `validate --all` as pre-phase). Engine rejects v0/v1 stems at parse time. **Body-sourced field validation**: Phase 1 validation now resolves fields with `source:` directives directly, making `required` and `enum` constraints apply to body-extracted values. Resolution order: frontmatter value (if present) takes precedence, falling back to body extraction via `extract.ResolveBodyValue` when the field has an `Extract` directive. This works independently of the derive pipeline; validation never depends on prior enrichment. Central resolution API in `resolver.go`: `StemChain`, `EffectiveSchema`, and `Resolve` return the stem chain, merged schema, and field provenance; `ClosestStem`/`RootMostStem` helpers provide explicit closest vs. root-most selection. `ResolveLayered(path, root, monotonic bool)` extends resolution with `LayeredResolution` (Layers + Conflicts); in monotonic mode detects type widening, required loosening, enum extension, severity loosening, and structural loosening as violations. `describe` and `explain` JSON output now include `layers` (ordered `.stem` chain) and `provenance` (field→source map) for consumer observability. `links.styles` selects which link styles are governed (default `[wikilink]`); `links.checks` (`resolve`/`anchors`/`encoding`) enables ADO code-wiki checks via `CheckLinks` (case-sensitive resolution, heading-slug anchors, `%20` encoding); `links.checks.cycles: true` opts `graph --check` into failing on link cycles (default: cycles are informational; `--fail-cycles` overrides). Graph respects styles via `FilterLinksByStyles`. - `internal/index/` — Directory scanner (respects `.stemignore`), file indexing, scope matching. - `stats` emits the v2 JSON contract `{version, kind, total}`; it reports only the total number of records after any `--where` filters. -- `internal/query/` — Query engine with declarative operators: `eq`, `ne`, `in`, `contains`, `exists`, `and`. Field shortcut resolution. Uses `expr-lang/expr` for expression evaluation. `field_check.go` provides `CheckFieldNames()` for pre-flight unknown-field detection with fuzzy suggestions. +- `internal/query/` — Query engine with declarative operators: `eq`, `ne`, `in`, `contains`, `exists`, `and`. Field shortcut resolution. Uses `expr-lang/expr` for expression evaluation. `field_check.go` provides `CheckFieldNames()` for pre-flight unknown-field detection with fuzzy suggestions; `sort.go` provides `ValidateSortKeys()`, which rejects a sort key naming a field nothing in scope can provide (SortRecords treats "absent on both records" as "equal", so an unknown key silently preserved scan order). - `internal/derive/` — Derivation engine using `expr-lang/expr`. Per-record derived fields, hierarchical aggregation (bottom-up from children to index files), builtin functions (slugify, lower, upper, trim, strlen, concat). `EnrichBuiltins` resolves the effective stem per-record via `ResolveForRecord`, so `source:` extracted fields (like `source: body.h1`) respect `match:` scopes — they apply only to matching records, not to the entire directory merge. - `internal/graph/` — Dependency graph from `[[wiki-links]]` in document bodies. Cycle detection, broken link analysis with fuzzy suggestions (up to 3 similar nodes), target resolution with basename fallback. DOT and Mermaid text output (Mermaid renders natively on GitHub and in most editors). - `internal/infer/` — Schema inference from existing documents (14 detectors: 12 data + 2 governance). `analyze` extracts records with the AST-enabled registry so section patterns, invariants, and formal dependencies are live at the command boundary. Analyzes frontmatter to detect field types, enum values, and required fields. `hierarchy.go` detects directory naming patterns (E##, F##, S###, T###) for hierarchical `.stem` generation with per-level field distribution. Body-aware detectors: `body_sections.go` (section patterns), `invariant_extraction.go` (INV\d+ extraction). Semantic extraction: `formal_dependency.go` (wiki-link deps), `traceability_links.go` (Contribuye a/Cubre/Satisface claims). Structural inference: `structural.go` (require_index, min/max_children, naming inconsistency detection across separate directory-name and record-file-stem populations). Governance detectors: `schema_coverage.go` (directories without .stem), `validation_gaps.go` (enum without values, untyped fields, sequence incomplete, required understatement). `scaffold.go` creates minimal `.stem` from observed frontmatter. `report.go` defines AnalyzeReport JSON schema (version: 1). `schema_gen.go` exports reusable schema generation services: `GenerateFlatSchema(ctx, dir, records, opts)` and `GenerateHierarchicalSchema(ctx, dir, records, opts)` return `*rules.StemFile` / `map[string]*rules.StemFile` without writing files; `init` command uses these instead of inline logic. @@ -57,6 +57,7 @@ Derivation evaluates per-record expressions from `.stem` `derive:` fields. Aggre - CLI commands call the Core Engine directly and emit stable versioned contracts. - Each JSON payload carries its own `version` for contract stability. Most commands use version 1; `tree` and `validate` use version 2. - `validate` emits exactly one envelope shape — `rootline/validate-batch` version 2 — for every invocation: one file, several files, `--all`, `--staged` with an empty index, and the corpus-scan failure path. Its six keys (`results`, `structural`, `stem_health`, `drift_warnings`, `notices`, `summary`) are always present, empty collections as `[]`. `results` holds documents only — directory verdicts from `structural:` rules moved to `structural[]` — so `summary.total` agrees with `query --count` on the same path; `.stem` diagnostics live in `stem_health` with severity `error`/`warn`/`info` and their own `stem_health_*_count` summary fields; run-level diagnostics (`scan_failed`, `schema_resolution_failed`, `stem_health_unavailable`, `no_records`) live in `notices`, keyed by a stable `code`. Stem health runs before the corpus scan and survives its failure, so a missing or unparseable `.stem` still emits JSON (with `stem-files-exist` or `yaml-valid`) instead of a raw Go error. Exit is non-zero on an invalid record, a structural error, a stem-health error, or an error notice; `--strict` adds warnings on all three axes; `info` never fails. See `docs/validate.md` for the version 1 → 2 upgrade table. +- `--where` and `--sort` field names are validated against one shared set, built by `knownWhereFields` in `cmd/rootline/filter.go`: query builtins (`path`, `body`, `type`, `sections`), every key the scanned corpus carries, and every `schema:`/`derive:`/`aggregate:` name the effective `.stem` chain declares. The union is deliberately generous — a false "unknown field" teaches callers to ignore the warning. It is computed from the corpus BEFORE `--where` narrows it, so a filter matching nothing cannot invalidate a valid `--sort` key, and it returns nil for a corpus with neither records nor schema (nothing to check against). An unknown `--where` field is a stderr WARNING with the exit code unchanged, on all five commands that accept `--where` (`query`, `stats`, `tree`, `graph`, `validate --all`) — `filterRecords` takes the warning writer as a parameter so it is the command's error stream, not a bare `os.Stderr`. An unknown `--sort` field is an ERROR, matching the treatment a bad sort direction already got. - `--output` is a validated enum, not a hint. `cmd/rootline/output.go` holds `advertisedFormats` (`json|jsonl|csv|table`) and `commandOutputFormats`, a per-command-path table of what each command actually implements; `rootPreflight` (root `PersistentPreRunE`) rejects an unknown value, then a value the command does not support, before the command body runs. `jsonl`/`csv` are `query --select` only — everywhere else they are rejected rather than downgraded to JSON, and on `tree`/`graph` the diagram is bound to `-o table` alone. A command with no entry in the table passes at runtime but fails `TestCommandOutputFormats_CoversEveryCommand`, so a new command cannot ship without declaring its formats (`formatAgnostic` is the explicit "ignores `--output`" value). `graph --check` rejects an explicitly-set `--output` (`cmd.Flags().Changed`), leaving the documented default invocation untouched. See `docs/output.md`. - `.stem` merge behavior is determined by YAML data type, not field names. - Version is injected via ldflags at build time (`cmd/rootline/root.go`). diff --git a/cmd/rootline/fieldcheck_test.go b/cmd/rootline/fieldcheck_test.go new file mode 100644 index 00000000..baf23c22 --- /dev/null +++ b/cmd/rootline/fieldcheck_test.go @@ -0,0 +1,167 @@ +package main + +import ( + "path/filepath" + "slices" + "strings" + "testing" + + "github.com/pablontiv/rootline/internal/extract" +) + +// A misspelled --where field is the highest-frequency user error on this CLI, +// and it used to be indistinguishable from "no records match": zero results, +// empty stderr, exit 0. Every command that accepts --where must say so. +func TestWhere_UnknownFieldWarnsOnEveryCommand(t *testing.T) { + docs := setupFormatProject(t) + + cases := []struct { + name string + args []string + }{ + {"query", []string{"query", docs, "--count", "--where", "estadoo == 'Pending'"}}, + {"stats", []string{"stats", docs, "--where", "estadoo == 'Pending'"}}, + {"tree", []string{"tree", docs, "--where", "estadoo == 'Pending'"}}, + {"validate", []string{"validate", "--all", docs, "--where", "estadoo == 'Pending'"}}, + {"graph", []string{"graph", docs, "--check", "--where", "estadoo == 'Pending'"}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + out, _ := runCmd(t, tc.args...) + if !strings.Contains(out, `unknown field "estadoo"`) { + t.Errorf("expected an unknown-field warning, got: %s", out) + } + if !strings.Contains(out, `did you mean "estado"`) { + t.Errorf("expected a fuzzy suggestion, got: %s", out) + } + }) + } +} + +// The warning must not become an error: a field absent from every record is a +// legal filter that yields zero matches, and pipelines depend on that. +func TestWhere_UnknownFieldDoesNotChangeExitCode(t *testing.T) { + docs := setupFormatProject(t) + + if _, err := runCmd(t, "query", docs, "--count", "--where", "estadoo == 'Pending'"); err != nil { + t.Fatalf("unknown --where field must warn, not fail: %v", err) + } +} + +// No false positives: a schema field, a derived/builtin field, and a +// frontmatter key that no .stem declares must all pass silently. +func TestWhere_KnownFieldsStaySilent(t *testing.T) { + docs := setupFormatProject(t) + + for _, where := range []string{ + "estado == 'Pending'", + "path != ''", + "body != ''", + } { + out, err := runCmd(t, "query", docs, "--count", "--where", where) + if err != nil { + t.Fatalf("--where %q: unexpected error: %v", where, err) + } + if strings.Contains(out, "unknown field") { + t.Errorf("--where %q: unexpected warning: %s", where, out) + } + } +} + +// A bad sort *direction* already exits 1. A bad sort *field* silently produced +// scan order, so the command validated half its input. +func TestSort_UnknownFieldErrors(t *testing.T) { + docs := setupFormatProject(t) + + _, err := runCmd(t, "query", docs, "--select", "path,estado", "--sort", "nosuch:asc") + if err == nil { + t.Fatal("expected an error for an unknown sort field, got none") + } + if !strings.Contains(err.Error(), "nosuch") { + t.Errorf("error = %v, want it to name the offending field", err) + } +} + +func TestSort_UnknownFieldSuggests(t *testing.T) { + docs := setupFormatProject(t) + + _, err := runCmd(t, "query", docs, "--select", "path,estado", "--sort", "estadoo:asc") + if err == nil { + t.Fatal("expected an error, got none") + } + if !strings.Contains(err.Error(), `did you mean "estado"`) { + t.Errorf("error = %v, want a fuzzy suggestion", err) + } +} + +// Sorting by a schema field, a builtin, and a field only some records carry +// must keep working. +func TestSort_KnownFieldsAccepted(t *testing.T) { + docs := setupFormatProject(t) + + for _, key := range []string{"estado:asc", "path:desc", "estado:asc,path:asc"} { + if _, err := runCmd(t, "query", docs, "--select", "path,estado", "--sort", key); err != nil { + t.Errorf("--sort %q: unexpected error: %v", key, err) + } + } +} + +// An empty result set must not make every field look unknown: the legal field +// names come from the scanned corpus, before --where narrows it. +func TestSort_ValidatesAgainstUnfilteredCorpus(t *testing.T) { + docs := setupFormatProject(t) + + _, err := runCmd(t, "query", docs, "--select", "path,estado", + "--where", "estado == 'NeverMatches'", "--sort", "estado:asc") + if err != nil { + t.Fatalf("a valid sort field must survive a filter that matches nothing: %v", err) + } +} + +// A corpus with no records and no schema has nothing to validate against; +// inventing verdicts there would be noise, not diagnosis. +func TestFieldCheck_EmptyCorpusIsSilent(t *testing.T) { + root := t.TempDir() + mustWriteFile(t, filepath.Join(root, ".stem"), []byte("version: 2\nroot: true\n"), 0o644) + + out, err := runCmd(t, "query", root, "--count", "--where", "whatever == 'x'") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if strings.Contains(out, "unknown field") { + t.Errorf("empty corpus should not warn, got: %s", out) + } +} + +func TestKnownWhereFields_UnionsRecordsSchemaAndBuiltins(t *testing.T) { + root := t.TempDir() + mustWriteFile(t, filepath.Join(root, ".stem"), []byte( + "version: 2\nroot: true\nschema:\n estado:\n type: enum\n values: [Pending]\n"+ + "derive:\n slug: slugify(path)\naggregate:\n total: count(children)\n"), 0o644) + + records := []*extract.Record{ + {Path: "a.md", Frontmatter: map[string]any{"owner": "pablo"}}, + {Path: "b.md", Derived: map[string]any{"computed": 1}}, + } + + got := knownWhereFields(records, root) + + for _, want := range []string{"path", "body", "type", "sections", "estado", "slug", "total", "owner", "computed"} { + if !slices.Contains(got, want) { + t.Errorf("knownWhereFields is missing %q; got %v", want, got) + } + } + if !slices.IsSorted(got) { + t.Errorf("knownWhereFields must be deterministic; got %v", got) + } +} + +func TestKnownWhereFields_NilWhenNothingIsDeclaredOrObserved(t *testing.T) { + root := t.TempDir() + mustWriteFile(t, filepath.Join(root, ".stem"), []byte("version: 2\nroot: true\n"), 0o644) + + if got := knownWhereFields(nil, root); got != nil { + t.Errorf("expected nil for a corpus with no records and no schema, got %v", got) + } +} diff --git a/cmd/rootline/filter.go b/cmd/rootline/filter.go index a109cf2d..90979088 100644 --- a/cmd/rootline/filter.go +++ b/cmd/rootline/filter.go @@ -3,17 +3,71 @@ package main import ( "context" "fmt" - "os" + "io" + "maps" + "slices" "strings" "github.com/pablontiv/rootline/internal/extract" "github.com/pablontiv/rootline/internal/query" + "github.com/pablontiv/rootline/internal/rules" ) +// whereEnvBuiltins are the names query.BuildEnv injects on every record +// regardless of schema or frontmatter. They are legal in a where expression +// even on a corpus where no document declares anything. +var whereEnvBuiltins = []string{"path", "body", "type", "sections"} + +// knownWhereFields is the set of names a where expression or a sort key may +// legally reference: the builtins, every key any record actually carries, and +// every field the effective schema declares — including derive: and aggregate: +// names, which a record only carries once the pipeline has populated them. +// +// The union is deliberately generous. A false "unknown field" on a name that +// does work is worse than the silence this replaces: it teaches callers to +// ignore the warning. +// +// It returns nil for a corpus with neither records nor schema. There is +// nothing to check against there, and every name would look wrong. +func knownWhereFields(records []*extract.Record, absRoot string) []string { + fields := make(map[string]bool) + + for _, rec := range records { + for k := range rec.Frontmatter { + fields[k] = true + } + for k := range rec.Derived { + fields[k] = true + } + } + + if entries, err := rules.WalkUp(absRoot); err == nil && len(entries) > 0 { + merged := rules.MergeStemFiles(entries) + for k := range merged.Schema { + fields[k] = true + } + for k := range merged.Derive { + fields[k] = true + } + for k := range merged.Aggregate { + fields[k] = true + } + } + + if len(fields) == 0 { + return nil + } + for _, b := range whereEnvBuiltins { + fields[b] = true + } + return slices.Sorted(maps.Keys(fields)) +} + // filterRecords applies where expressions to records, returning only those that match. // Multiple wheres are combined with AND. Empty wheres returns all records (passthrough). -// If knownFields is non-nil, unknown field names in the expression emit warnings to stderr. -func filterRecords(ctx context.Context, records []*extract.Record, wheres []string, knownFields []string) ([]*extract.Record, error) { +// If knownFields and warn are both non-nil, unknown field names in the +// expression are reported on warn. +func filterRecords(ctx context.Context, records []*extract.Record, wheres []string, knownFields []string, warn io.Writer) ([]*extract.Record, error) { // Filter empty strings — StringArrayVar may produce [""] on cobra re-execution. var cleaned []string for _, w := range wheres { @@ -27,10 +81,9 @@ func filterRecords(ctx context.Context, records []*extract.Record, wheres []stri whereExpr := strings.Join(cleaned, " && ") - if len(knownFields) > 0 { - warnings := query.CheckFieldNames(whereExpr, knownFields) - for _, w := range warnings { - fmt.Fprintf(os.Stderr, "warning: %s\n", w.Message) + if len(knownFields) > 0 && warn != nil { + for _, w := range query.CheckFieldNames(whereExpr, knownFields) { + _, _ = fmt.Fprintf(warn, "warning: %s\n", w.Message) } } diff --git a/cmd/rootline/filter_test.go b/cmd/rootline/filter_test.go index 36894771..2a48545e 100644 --- a/cmd/rootline/filter_test.go +++ b/cmd/rootline/filter_test.go @@ -14,7 +14,7 @@ func TestFilterRecords_MatchSubset(t *testing.T) { {Path: "c.md", Frontmatter: map[string]any{"estado": "Pending"}}, } - filtered, err := filterRecords(context.Background(), records, []string{"estado == 'Pending'"}, nil) + filtered, err := filterRecords(context.Background(), records, []string{"estado == 'Pending'"}, nil, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -33,7 +33,7 @@ func TestFilterRecords_NoMatch(t *testing.T) { {Path: "a.md", Frontmatter: map[string]any{"estado": "Pending"}}, } - filtered, err := filterRecords(context.Background(), records, []string{"estado == 'Completed'"}, nil) + filtered, err := filterRecords(context.Background(), records, []string{"estado == 'Completed'"}, nil, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -47,7 +47,7 @@ func TestFilterRecords_InvalidExpr(t *testing.T) { {Path: "a.md", Frontmatter: map[string]any{"estado": "Pending"}}, } - _, err := filterRecords(context.Background(), records, []string{"== bad syntax"}, nil) + _, err := filterRecords(context.Background(), records, []string{"== bad syntax"}, nil, nil) if err == nil { t.Fatal("expected error for invalid expression") } @@ -59,7 +59,7 @@ func TestFilterRecords_EmptyWheres(t *testing.T) { {Path: "b.md", Frontmatter: map[string]any{"estado": "Completed"}}, } - filtered, err := filterRecords(context.Background(), records, nil, nil) + filtered, err := filterRecords(context.Background(), records, nil, nil, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -68,7 +68,7 @@ func TestFilterRecords_EmptyWheres(t *testing.T) { } // Also test with empty slice. - filtered, err = filterRecords(context.Background(), records, []string{}, nil) + filtered, err = filterRecords(context.Background(), records, []string{}, nil, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -77,7 +77,7 @@ func TestFilterRecords_EmptyWheres(t *testing.T) { } // Also test with empty string entries. - filtered, err = filterRecords(context.Background(), records, []string{"", ""}, nil) + filtered, err = filterRecords(context.Background(), records, []string{"", ""}, nil, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -93,7 +93,7 @@ func TestFilterRecords_MultipleWheres(t *testing.T) { {Path: "c.md", Frontmatter: map[string]any{"estado": "Completed", "tipo": "test"}}, } - filtered, err := filterRecords(context.Background(), records, []string{"estado == 'Pending'", "tipo == 'test'"}, nil) + filtered, err := filterRecords(context.Background(), records, []string{"estado == 'Pending'", "tipo == 'test'"}, nil, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/cmd/rootline/graph.go b/cmd/rootline/graph.go index 7b3e5f8f..003753ea 100644 --- a/cmd/rootline/graph.go +++ b/cmd/rootline/graph.go @@ -82,7 +82,7 @@ func runGraph(cmd *cobra.Command, args []string) error { derive.EnrichBuiltinsSimple(ctx, records, absRoot) // Apply --where filter. - records, err = filterRecords(ctx, records, graphWhere, nil) + records, err = filterRecords(ctx, records, graphWhere, knownWhereFields(records, absRoot), cmd.ErrOrStderr()) if err != nil { return fmt.Errorf("filtering records: %w", err) } diff --git a/cmd/rootline/query.go b/cmd/rootline/query.go index 524700d8..b0f40500 100644 --- a/cmd/rootline/query.go +++ b/cmd/rootline/query.go @@ -127,8 +127,17 @@ func runQuery(cmd *cobra.Command, args []string) error { Limit: queryLimit, } + // The legal field names come from the scanned corpus, before --where + // narrows it: a filter that matches nothing must not make every field name + // look invented. + known := knownWhereFields(records, absRoot) + + if err := query.ValidateSortKeys(sortKeys, known); err != nil { + return err + } + // Filter records using shared helper. - filtered, err := filterRecords(ctx, records, queryWhere, nil) + filtered, err := filterRecords(ctx, records, queryWhere, known, cmd.ErrOrStderr()) if err != nil { return fmt.Errorf("filtering records: %w", err) } diff --git a/cmd/rootline/stats.go b/cmd/rootline/stats.go index 99764a8a..05d3d49b 100644 --- a/cmd/rootline/stats.go +++ b/cmd/rootline/stats.go @@ -60,7 +60,7 @@ func runStats(cmd *cobra.Command, args []string) error { derive.AggregateAllSimple(ctx, records, absRoot) // Apply --where filter. - records, err = filterRecords(ctx, records, statsWhere, nil) + records, err = filterRecords(ctx, records, statsWhere, knownWhereFields(records, absRoot), cmd.ErrOrStderr()) if err != nil { return fmt.Errorf("filtering records: %w", err) } diff --git a/cmd/rootline/tree.go b/cmd/rootline/tree.go index 2b83ece7..60713d6c 100644 --- a/cmd/rootline/tree.go +++ b/cmd/rootline/tree.go @@ -117,7 +117,7 @@ func runTree(cmd *cobra.Command, args []string) error { derive.AggregateAllSimple(ctx, records, absRoot) // Apply --where filter. - records, err = filterRecords(ctx, records, treeWhere, nil) + records, err = filterRecords(ctx, records, treeWhere, knownWhereFields(records, absRoot), cmd.ErrOrStderr()) if err != nil { return fmt.Errorf("filtering records: %w", err) } diff --git a/cmd/rootline/validate.go b/cmd/rootline/validate.go index fc35ea13..4db94ead 100644 --- a/cmd/rootline/validate.go +++ b/cmd/rootline/validate.go @@ -208,7 +208,7 @@ func runValidateAll(cmd *cobra.Command, args []string) error { derive.AggregateAllSimple(ctx, records, root) // Apply --where filter. - records, err = filterRecords(ctx, records, validateWhere, nil) + records, err = filterRecords(ctx, records, validateWhere, knownWhereFields(records, root), cmd.ErrOrStderr()) if err != nil { return fmt.Errorf("filtering records: %w", err) } diff --git a/docs/query.md b/docs/query.md index d746f982..87c633b8 100644 --- a/docs/query.md +++ b/docs/query.md @@ -32,6 +32,15 @@ Sort type detection per field: Missing/nil values always sort last, regardless of direction. Sort applies after filtering and before limit. +Unlike `--where`, an unknown `--sort` field is an **error**, matching the treatment a bad direction already got: + +```console +$ rootline query docs/ --sort "estadoo:asc" +Error: unknown sort field "estadoo": no record carries it and no .stem in scope declares it (did you mean "estado"?) +``` + +An unsortable key cannot produce a defensible result — every comparison falls through, the output is silently in scan order, and it is indistinguishable from a correct sort. The legal names are the same set `--where` checks against, taken from the corpus **before** `--where` narrows it, so a filter that matches nothing does not invalidate a valid sort key. + ### Link Traversal Predicates Query documents based on their link relationships using `--has-inbound` and `--has-outbound`: @@ -150,6 +159,12 @@ The heading key must match the heading text exactly, including the `#` prefix an > **Field Warnings**: Unknown field names in `--where` expressions emit warnings to stderr with fuzzy suggestions (e.g., `warning: unknown field "estdo" in where expression (did you mean "estado"?)`). Queries still execute — warnings are informational only. +The warning fires on every command that accepts `--where`: `query`, `stats`, `tree`, `graph` and `validate --all`. Without it a misspelled field is indistinguishable from "no records match" — zero results, empty stderr, exit 0 — which reads as a green check in CI. + +A field name is considered known when it is a query builtin (`path`, `body`, `type`, `sections`), a key any record in the scanned corpus carries, or a field the effective `.stem` chain declares — including `derive:` and `aggregate:` names. The union is deliberately generous: a false warning on a name that does work would teach callers to ignore the warning. On a corpus with neither records nor schema there is nothing to check against, and nothing is reported. + +It stays a warning, not an error. A field absent from every record is a legal filter that yields zero matches, and pipelines depend on that exit code. + ## Operators Standard expr-lang operators apply: diff --git a/internal/query/sort.go b/internal/query/sort.go index 2b58a607..dafe59c4 100644 --- a/internal/query/sort.go +++ b/internal/query/sort.go @@ -2,10 +2,12 @@ package query import ( "fmt" + "slices" "sort" "strconv" "strings" + "github.com/pablontiv/picokit/fuzzy" "github.com/pablontiv/rootline/internal/extract" "github.com/pablontiv/rootline/internal/rules" ) @@ -62,6 +64,34 @@ func ParseSortKeys(spec string) ([]SortKey, error) { return keys, nil } +// ValidateSortKeys rejects a sort key naming a field nothing in scope can +// provide. +// +// SortRecords treats "absent on both records" as "equal", so an unknown key +// left every comparison falling through and sort.SliceStable preserved scan +// order: a typo produced plausible, unsorted output at exit 0, while a bad +// direction already exited 1. The command validated half its input. +// +// knownFields is the same set the where-expression check uses — builtins, +// every key the corpus carries, and every field the effective schema declares. +// An empty set means there was nothing to validate against (no records, no +// schema), and every key passes rather than every key failing. +func ValidateSortKeys(keys []SortKey, knownFields []string) error { + if len(keys) == 0 || len(knownFields) == 0 { + return nil + } + for _, key := range keys { + if slices.Contains(knownFields, key.Field) { + continue + } + if suggestion := fuzzy.Match(key.Field, knownFields); suggestion != "" { + return fmt.Errorf("unknown sort field %q: no record carries it and no .stem in scope declares it (did you mean %q?)", key.Field, suggestion) + } + return fmt.Errorf("unknown sort field %q: no record carries it and no .stem in scope declares it", key.Field) + } + return nil +} + // SortRecords sorts records in-place by the given sort keys. // schema may be nil if no enum ordering is needed. // Uses sort.SliceStable for deterministic ordering of equal elements. diff --git a/internal/query/sort_test.go b/internal/query/sort_test.go index d2f1d0bf..f007869a 100644 --- a/internal/query/sort_test.go +++ b/internal/query/sort_test.go @@ -1,6 +1,7 @@ package query import ( + "strings" "testing" "github.com/pablontiv/rootline/internal/extract" @@ -389,3 +390,45 @@ func TestSortRecords_NumericStringValues(t *testing.T) { } } } + +func TestValidateSortKeys_AcceptsKnownFields(t *testing.T) { + known := []string{"estado", "path", "titulo"} + keys := []SortKey{{Field: "estado"}, {Field: "path", Desc: true}} + + if err := ValidateSortKeys(keys, known); err != nil { + t.Errorf("unexpected error: %v", err) + } +} + +func TestValidateSortKeys_RejectsUnknownFieldWithSuggestion(t *testing.T) { + err := ValidateSortKeys([]SortKey{{Field: "estadoo"}}, []string{"estado", "path"}) + if err == nil { + t.Fatal("expected an error for an unknown sort field") + } + if !strings.Contains(err.Error(), `"estadoo"`) { + t.Errorf("error = %v, want it to name the offending field", err) + } + if !strings.Contains(err.Error(), `did you mean "estado"`) { + t.Errorf("error = %v, want a fuzzy suggestion", err) + } +} + +func TestValidateSortKeys_RejectsUnknownFieldWithoutSuggestion(t *testing.T) { + err := ValidateSortKeys([]SortKey{{Field: "zzzzzzzz"}}, []string{"estado", "path"}) + if err == nil { + t.Fatal("expected an error for an unknown sort field") + } + if strings.Contains(err.Error(), "did you mean") { + t.Errorf("error = %v, want no suggestion when nothing is close", err) + } +} + +// Nothing to validate against is not the same as everything being wrong. +func TestValidateSortKeys_EmptyKnownSetPasses(t *testing.T) { + if err := ValidateSortKeys([]SortKey{{Field: "anything"}}, nil); err != nil { + t.Errorf("unexpected error: %v", err) + } + if err := ValidateSortKeys(nil, []string{"estado"}); err != nil { + t.Errorf("unexpected error: %v", err) + } +}