From 9810a755649c3ffa06320833767496f02801edb8 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 26 Aug 2026 19:54:26 -0400 Subject: [PATCH] =?UTF-8?q?M311=20step=202=20=E2=80=94=20the=20migration?= =?UTF-8?q?=20that=20lifts=20the=20notes=20already=20written?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 1 made new saves lift; this does the ones already on disk, so a note stops showing its tag twice without having to be opened. Same rule, and a FROZEN copy of it — `split_body_tags` is deliberately not imported, on 0027's principle that a migration has to keep producing what it produced the day it ran. If the app's rule is ever loosened, this file must not loosen with it and start eating prose it previously left alone. `_display_title` is inlined for the same reason, and recomputed only for a note whose body actually moved: a note named after its `#todo` line needs a new name. The label rows graduate in the same transaction, and that is not cosmetic. A `via_tag` row claims "backed by text still in the body", and reconcile detaches any row it cannot find a `#tag` for — so leaving them true would lose every lifted tag on the note's next save. Flipping them to false is also what makes the chip's × appear, which is now the only way to remove a tag whose text is gone. `updated_at` is left alone so a client holding an unpushed edit still wins under LWW. The `sync_revision` trigger does fire, which is wanted here: unlike 0027 the clients do NOT yet apply this rule locally, so the server's copy is the only correct one until step 3. The downgrade is empty and says why. It cannot restore the deleted lines — nothing distinguishes one this migration removed from one that was never there — and flipping the rows back would be actively harmful, since the text that flag claims backs them is gone and the next save would then detach the label for real. Tested on the ten cases that matter, three of which are prose that must come back byte-identical. The test pins the frozen copy against fixed expectations rather than against the app's rule — they are allowed to diverge later, which is the whole point of freezing one. Co-Authored-By: Claude Opus 5 --- alembic/versions/0028_lift_standalone_tags.py | 168 ++++++++++++++++++ tests/test_notes.py | 44 +++++ 2 files changed, 212 insertions(+) create mode 100644 alembic/versions/0028_lift_standalone_tags.py diff --git a/alembic/versions/0028_lift_standalone_tags.py b/alembic/versions/0028_lift_standalone_tags.py new file mode 100644 index 0000000..fc9663c --- /dev/null +++ b/alembic/versions/0028_lift_standalone_tags.py @@ -0,0 +1,168 @@ +"""lift standalone #tags out of note bodies + +Revision ID: 0028 +Revises: 0027 +Create Date: 2026-08-26 + +M311. A `#tag` was being shown twice — once as the text you typed and once as a chip — +and with the chip moved to the top of the card the text is redundant. This removes it, +but only from notes where the tag was standing on its own. + +## This migration rewrites note bodies + +The rule is deliberately narrow, and the same one `notes/tags.py:split_body_tags` +applies from here on: + + * A line containing nothing but tags and whitespace is REMOVED. + * Every other line is left exactly as written. + +So `#todo` on its own line goes, and `remember to call #mom tomorrow` does not. The +looser reading — also stripping a trailing tag off a prose line — was rejected because +the text does not say which kind it is: `buy milk #grocery` is filing, `remember to +call #mom` is the sentence's object, and lifting the second leaves "remember to call". +Rewriting somebody's words to save a duplicate chip is a bad trade, and a migration is +the worst possible place to make it. + +Two guards, both of which cost a note nothing: + + * A line inside a ``` fence is never touched. A `#tag` there is a shell comment in a + snippet somebody pasted, and deleting it would eat a line of their example. + * A note that is NOTHING but tags keeps its text. Lifting would leave a blank card, + which is worse than the duplication this fixes. + +## The label rows have to graduate in the same transaction + +A `via_tag` row means "this label is backed by text still in the body". Once the text +is gone that is false, and leaving it true is not cosmetic: `_lift_and_reconcile_tags` +detaches any `via_tag` row it cannot find a `#tag` for, so the note would lose the tag +on its very next save. The flip to `via_tag = false` is what makes the label the record +instead — and what makes the chip's × appear in both editors, which is now the only way +to remove a tag whose text no longer exists. + +## The transform is inlined, like 0027's + +`split_body_tags` is deliberately NOT imported. A migration has to keep producing what +it produced the day it ran; if the app's rule is ever loosened, this file must not +loosen with it and start eating prose it previously left alone. + +`_display_title` is inlined for the same reason, and is only recomputed for a note whose +body actually moved — a note named after a `#todo` line needs a new name, and reading it +from the app would couple this migration to a rule that has already changed once (M13). + +## `updated_at` is left alone, and that is load-bearing + +Raw SQL, so SQLAlchemy's `onupdate` never fires. A client holding an UNPUSHED body edit +keeps the newer `updated_at`, so when it pulls the migrated note last-write-wins keeps +its edit instead of the migration silently winning. + +The `sync_revision` trigger (migration 0015) does fire, so every rewritten note becomes +pullable once and clients converge on the server's text. That is wanted here: unlike +0027, the clients do NOT yet apply this rule locally, so the server's copy is the only +correct one until they do. + +## The downgrade is not a true inverse, and says so + +It cannot be. Nothing distinguishes a `#todo` line this migration deleted from one that +was never there, and putting one back would be guessing at where in the note it went. + +Nothing is lost, though, which is why that is acceptable: the tag still exists as a +label on the note, and the chip still shows it. What a downgrade cannot restore is the +DUPLICATE — which is the thing this migration set out to remove. Rolling the rows back +to `via_tag = true` would be actively harmful: the text that flag claims to be backed by +is gone, so the next save would detach the label and lose the tag for real. So the +downgrade leaves both alone. The real rollback is a database restore. +""" +import re + +import sqlalchemy as sa +from alembic import op + +revision = "0028" +down_revision = "0027" +branch_labels = None +depends_on = None + +# Frozen copies. See "The transform is inlined" above — these must not follow the app. +_TAG_RE = re.compile(r"(?:^|(?<=\s))#(\w[\w-]*)") +_FENCE_RE = re.compile(r"^\s*(?:```|~~~)") +_TASK_RE = re.compile(r"^(?P\s*)(?P[-*]) +\[(?P[ xX])\](?: +(?P.*))?$") +_DISPLAY_TITLE_CAP = 200 + + +def _is_tag(name: str) -> bool: + """A tag must contain a letter, so #2024 and #_ are not tags — and a line holding + only those is therefore not a tag-only line and is left alone.""" + return any(c.isalpha() for c in name) + + +def _split(body: str) -> tuple[list[str], str]: + """(standalone tag names, body with their lines removed).""" + standalone: list[str] = [] + kept: list[str] = [] + in_fence = False + for line in body.split("\n"): + if _FENCE_RE.match(line): + in_fence = not in_fence + kept.append(line) + continue + matches = [m for m in _TAG_RE.finditer(line) if _is_tag(m.group(1))] + remainder = line + for m in reversed(matches): + remainder = remainder[: m.start()] + remainder[m.end() :] + if in_fence or not matches or remainder.strip(): + kept.append(line) + else: + standalone.extend(m.group(1) for m in matches) + lifted = re.sub(r"\n{3,}", "\n\n", "\n".join(kept)).strip("\n") + if body.strip() and not lifted.strip(): + return [], body # nothing but tags: keep the note readable + # A tag still written in prose somewhere keeps its text, so it stays derived. + still_in_prose = {m.group(1).lower() for m in _TAG_RE.finditer(lifted) if _is_tag(m.group(1))} + return [n for n in standalone if n.lower() not in still_in_prose], lifted + + +def _display_title(body: str) -> str: + for line in body.splitlines(): + stripped = line.strip() + match = _TASK_RE.match(stripped) + text = (match.group("text") or "") if match else stripped + text = text.strip() + if text: + return text[:_DISPLAY_TITLE_CAP] + return "" + + +def upgrade(): + bind = op.get_bind() + rows = bind.execute(sa.text("SELECT id, body FROM notes WHERE body LIKE '%#%'")).fetchall() + + flip = sa.text( + "UPDATE note_labels nl SET via_tag = false " + "FROM labels l " + "WHERE nl.label_id = l.id AND nl.note_id = :nid AND nl.via_tag = true " + "AND lower(l.name) IN :names" + ).bindparams(sa.bindparam("names", expanding=True)) + + for note_id, body in rows: + if not body: + continue + standalone, lifted = _split(body) + if lifted != body: + bind.execute( + sa.text("UPDATE notes SET body = :body, display_title = :title WHERE id = :id"), + {"body": lifted, "title": _display_title(lifted), "id": note_id}, + ) + # Even when the body did not move, a tag can be standalone only in the sense + # that its line was already removed by an earlier pass — so the flip is driven + # by the tag list, not by whether the text changed. + if standalone: + bind.execute(flip, {"nid": note_id, "names": [n.lower() for n in standalone]}) + + +def downgrade(): + """Deliberately empty — see the module docstring. + + Restoring the deleted lines would be guessing, and flipping the rows back to + `via_tag = true` would be worse than doing nothing: the text that flag claims backs + them is gone, so the next save would detach the label and lose the tag for real. + """ diff --git a/tests/test_notes.py b/tests/test_notes.py index c1d3940..2ebdd26 100644 --- a/tests/test_notes.py +++ b/tests/test_notes.py @@ -220,6 +220,50 @@ def test_split_body_tags_keeps_a_tag_derived_when_prose_still_carries_it(): assert body == "remember the #todo list" +def test_migration_0028_lifts_exactly_what_the_app_lifts_today(): + """The 0028 data migration rewrites note bodies, and that is not undoable. + + It carries its OWN frozen copy of the rule rather than importing + `split_body_tags`, on 0027's principle that a migration must keep producing what + it produced the day it ran. This does not assert the two agree — they are allowed + to diverge later, which is the entire point of freezing one. It pins the frozen + copy against fixed expectations, so nobody can "tidy" it into eating prose. + """ + import importlib.util + from pathlib import Path + + path = Path(__file__).resolve().parents[1] / "alembic" / "versions" / "0028_lift_standalone_tags.py" + spec = importlib.util.spec_from_file_location("migration_0028", path) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + + # Lifted: the tag was the whole line. + assert mod._split("#todo\nreorganize the homepage") == (["todo"], "reorganize the homepage") + assert mod._split("needs a tauri app\n#todo") == (["todo"], "needs a tauri app") + assert mod._split("#todo #work\nreal text") == (["todo", "work"], "real text") + assert mod._split("foo\n\n#todo\n\nbar") == (["todo"], "foo\n\nbar") + + # Untouched: prose. Getting any of these wrong destroys somebody's words. + for prose in ("remember to call #mom tomorrow", "buy milk #grocery", "#2024\nreal"): + assert mod._split(prose) == ([], prose), prose + + # Untouched: a tag inside a fence is a shell comment in somebody's snippet. + fenced = "code:\n```\n#!/bin/sh\n#deploy\n```\ndone" + assert mod._split(fenced) == ([], fenced) + + # Untouched: a note that is nothing but tags would be blanked. + assert mod._split("#todo") == ([], "#todo") + + # Not flipped: the tag is still written in prose, so its text still backs it and + # it must stay derived — flipping it would be claiming otherwise. + assert mod._split("#todo\nremember the #todo list") == ([], "remember the #todo list") + + # The name has to move with the body, or a note is titled after a deleted line. + assert mod._display_title("reorganize the homepage") == "reorganize the homepage" + assert mod._display_title("- [ ] milk\n- [ ] eggs") == "milk" + assert mod._display_title("") == "" + + def test_parse_tags(): assert parse_tags("buy milk #groceries and #to-do now") == ["groceries", "to-do"] # case-insensitive dedup, first spelling wins