Skip to content

wren context upgrade silently deletes v1 views that the loader dropped #2687

Description

@goldmedal

Summary

wren context upgrade writes the v1→v2 view directories from _load_views_v1(), which drops malformed entries, and then unconditionally unlink()s views.yml. Anything the loader dropped is therefore deleted from disk with no error, no warning, and exit code 0. In the worst case the entire views.yml is destroyed.

Dropping malformed entries in the loader is correct and intentional (validate_project is the designated reporter). Doing it on a migration path that deletes the source file is data loss.

Measured on main @ ce9513d6, wren 0.13.3.

Repro A — individual views vanish

# views.yml
views:
  - name: kept
    statement: SELECT 1
  - statement: SELECT id FROM orders     # no name
  - just_a_bare_string                   # not a mapping
$ wren context upgrade --path "$P"
Upgrading project from schema_version 1 -> 5...
  + models/orders/metadata.yml
  + views/kept/metadata.yml
  ...
  - views.yml
  * wren_project.yml (schema_version 1 -> 5)

Upgrade complete. Run `wren context validate` to check the result.
$ echo $?
0
$ ls views/
kept

SELECT id FROM orders and the bare string are gone permanently. The plan output never mentions them.

Repro B — the whole file is destroyed

A views.yml hand-written as a mapping instead of a list — a very plausible edit:

# views.yml
views:
  revenue:
    statement: SELECT sum(amount) FROM orders
  churn:
    statement: SELECT 1
$ wren context upgrade --path "$P"
  ...
  - views.yml
Upgrade complete. Run `wren context validate` to check the result.
$ echo $?
0
$ ls views/ 2>&1
ls: views/: No such file or directory

Both view definitions are gone and no views/ directory was created. Only git can recover them.

Root cause

  • context.py:612-614_load_views_v1() returns [] for a non-list views: value and filters non-mapping entries out of the list.
  • context.py:1708-1711_apply_v1_to_v2() iterates that already-filtered list and additionally continues on any view without a name.
  • context.py:1734-1737views.yml is then unlinked regardless of how many views were actually written.

Why nothing warns you

wren context validate does report the underlying problems on the v1 project and exits 1:

[ERROR] views.yml > views[2]: view entry must be a mapping, got str
[ERROR] views/views[1]/metadata.yml: view missing 'name'

But wren context upgrade does not run validation, and --dry-run gives no hint either — it prints Would create: views/kept/metadata.yml / Would delete: views.yml with nothing to indicate that two views are being dropped. A user reading the plan cannot tell the difference between this and a clean migration.

Expected

wren context upgrade should never delete a source file whose content it did not carry across. Either:

  1. Abort in the preflight. _plan_v1_to_v2() already rejects other unmigratable input (e.g. non-string view statements) before the first write. Extending it to reject malformed views.yml shape, non-mapping entries, and nameless views would fit that shape and keep the source intact.
  2. Don't filter on the migration path. Have _apply_v1_to_v2() read the raw YAML rather than going through the normalising loader, so migration is byte-preservingly lossless and validate_project keeps reporting the mistake afterwards in the v2 layout.

Option 2 is the more general fix: it closes this class of bug for every field at once, rather than requiring a new preflight rule each time a loader learns to filter something.

Scope note

This is the same bug class as the model columns case currently being fixed in #2614, but for views, and it is already live on main#2614's head and main behave identically here. Whichever way #2614 lands, the views path will still need this, so it is worth tracking separately.

Cubes are not affected: _load_cubes_v1() leaves a non-mapping cube file's source in place.

Minor, possibly separate

wren context validate prints two [ERROR] lines above and then summarises as 3 warning(s), 0 errors. The exit code is correct (1), so this is a cosmetic tally bug in the summary line, but it does make the errors easy to miss.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingcore

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions