Skip to content

Commit 42f8c73

Browse files
authored
feat(scripts): one Sieve generator, and the preview now comes from it (#61)
* feat(scripts): one Sieve generator, and the preview now comes from it `previewRule` in scriptDocument.js was a second implementation of SieveGenerator. Both modules said so in a comment — "it is a second implementation of the backend generator and the two must agree", "it must agree with the backend generator" — and nothing checked it. Five divergences had shipped: 1. dropped `negate`, so a NOT condition previewed as its opposite 2. no quote escaping 3. a disabled rule shown live, with no ## anywhere 4. no `# --- name ---` line, the only place a Rule's name is stored 5. NOTHING at all for a Rule whose last Condition was deleted, while a save writes `if anyof ( ) {` — Sieve a real server refuses The fifth is the worst: the editor showed an empty preview for the one state where the user most needed to be told something was wrong. POST /api/scripts/preview renders one Rule and answers. It depends on get_session and NOT on get_script_store, so it authenticates without opening a ManageSieve connection — the editor calls it on a keystroke timer, and a connection per keystroke is a denial of service aimed at the user's own mail server. That guard is a test, and the test is mutation-checked: swapping the dependency makes it fail, and a second test proves the sabotage bites on a route that DOES take the store. Save and preview now go through one function. `generate` used to inline the `## `-prefixing for a disabled Rule; that is `SieveGenerator .generate_entry` now, and both callers use it. `generate_rule` and `rule_from_json` are the module-level rule-sized entry points. THE TEST THAT COULD NOT EXIST BEFORE: preview a Rule, save that same Rule, and require the preview to be a literal substring of what the store then holds — across nine shapes including all five divergences. An agreement test needs one side to be authoritative; there was no such side while there were two generators. previewRule, renderTest, renderAction and the frontend `quote` are deleted, with their 93 lines of tests. Deleting the duplicate beats testing it. In their place scriptDocument exports `entryToWire`, which `toWire` now maps over — the preview must post the SAME projection a save posts, or it is previewing something else again. The editor debounces at 200ms and carries a sequence number, so a slow answer for an older Rule cannot overwrite a newer one, and it says "preview unavailable" rather than leaving the previous rule's Sieve on screen next to the rule being edited. Route inventory updated in both of its lists. A script may still be CALLED "preview" — pinned, because `POST /{name}` does not exist today and the day it does the shadowing would be silent. 1017 backend tests, 54 frontend, svelte-check 0/0, ruff clean. Refs areyousievious-8fg.17 * style(frontend): the blank lines the review counted Cosmetic only — doubled blank lines either side of the entryToWire block. Refs areyousievious-8fg.17 * perf(frontend): preview only when the selected rule actually changed Raised in review of #61, and my comment was the weaker claim: it said the trigger fires on each keystroke, when it fires on ANY document mutation. `rules` is rebuilt whenever `script` is reassigned, so adding, deleting or reordering some OTHER rule rescheduled a preview of the selected one and spent a request to be told the same bytes it was already showing. Comparing the wire payload rather than the array reference makes those free, and covers the keystroke that leaves wire content unchanged too. The payload compared is the one that gets posted, so the two cannot drift. An error clears the remembered payload, so the next mutation retries rather than matching it and leaving the message up for good. The comment now says what actually happens. Not unit-tested: the logic is inside a .svelte file and this repo has no component test harness. svelte-check, vitest and the build are clean. Refs areyousievious-8fg.17
1 parent 0f7037d commit 42f8c73

16 files changed

Lines changed: 691 additions & 218 deletions

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
1919

2020
### Added
2121

22+
- `POST /api/scripts/preview` renders one Rule through the backend generator, and the SPA's duplicate generator (`previewRule`) is deleted. The preview is now the bytes a save writes, asserted as such; the duplicate had diverged five ways, including showing nothing for a Rule whose last Condition was deleted while a save wrote invalid Sieve (areyousievious-8fg.17)
23+
2224
- GitHub Actions CI workflow (`.github/workflows/ci.yml`): runs pytest and frontend build on every push and pull request (P1)
2325
- Sieve parser regression test suite (`backend/tests/`) covering round-trip stability, else/elsif handling, address-part/`:comparator` parsing, and ReDoS budget (Phase CP1)
2426
- Sieve fixture corpus (`backend/test_scripts/`): twelve hand-written one-construct fixtures plus sievelib's parser corpus vendored under MIT in `vendor/`, with a per-fixture recognition census and a pinned recogniser-reach total (areyousievious-8fg.3)

backend/AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ FastAPI application that serves the Svelte SPA as static files and provides a RE
3030
## Subdirectories
3131
| Directory | Purpose |
3232
|-----------|---------|
33-
| `routers/` | One module per URL area: `auth`, `scripts`, `folders`, `health`, `static`. Do NOT cross-import between routers — shared helpers belong in `dependencies.py` |
33+
| `routers/` | One module per URL area: `auth`, `scripts`, `folders`, `health`, `static`. Do NOT cross-import between routers — shared helpers belong in `dependencies.py`. `POST /api/scripts/preview` is the one script route that takes `get_session` and NOT `get_script_store`: it renders a Rule through `generate_rule` without dialling, because the editor calls it on a keystroke timer |
3434
| `tests/` | 30 pytest files plus a shared conftest.py and `fakes.py` (in-memory ScriptStore/FolderStore), mostly regression locks tied to a bead id in the module docstring |
3535
| `test_scripts/` | The Sieve fixture corpus: three captured real-world scripts, twelve hand-written one-construct files, and `vendor/` — sievelib's own parser corpus under MIT, regenerated by `tools/vendor-sievelib-corpus.py` (see `test_scripts/AGENTS.md`) |
3636

backend/api_models.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,19 @@ class CreateFolderRequest(BaseModel):
179179
name: str = Field(min_length=1, max_length=200)
180180

181181

182+
class PreviewRequest(BaseModel):
183+
"""One Rule to render as Sieve (areyousievious-8fg.18 vocabularies apply).
184+
185+
Deliberately one Rule and not a whole script: the editor previews the rule
186+
the user is looking at, and asking for a script would make the endpoint
187+
need `requires`, which is a property of the script rather than of any Rule.
188+
"""
189+
190+
model_config = _STRICT
191+
192+
rule: RuleDTO
193+
194+
182195
# ── Response models ──
183196

184197

@@ -224,3 +237,10 @@ class ScriptResponse(BaseModel):
224237
class ScriptRawResponse(BaseModel):
225238
name: str
226239
content: str
240+
241+
242+
class PreviewResponse(BaseModel):
243+
"""POST /api/scripts/preview — the exact bytes a save would write for this
244+
Rule, minus the script-level `require` line."""
245+
246+
sieve: str

backend/routers/scripts.py

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
"""
22
Sieve script router (areyousievious-u40 split from app.py).
33
4-
Owns the nine /api/scripts/* endpoints that read, write, import,
4+
Owns the ten /api/scripts/* endpoints that read, write, import,
55
export, activate, and delete Sieve scripts via ManageSieve.
66
7+
`preview` is the one that does not touch ManageSieve at all: it renders a
8+
Rule with the same generator a save uses and answers. It exists so the SPA
9+
does not have to carry a second implementation of that generator, which it
10+
did, and which had diverged five ways (areyousievious-8fg.17).
11+
712
The import size cap comes from `request.app.state.settings.max_body_bytes`,
813
the same value the body-size middleware uses. It was a local constant here,
914
which meant raising AYS_MAX_BODY_BYTES moved one limit and not the other.
@@ -15,20 +20,25 @@
1520

1621
from api_models import (
1722
OkResponse,
23+
PreviewRequest,
24+
PreviewResponse,
1825
SaveRawRequest,
1926
SaveScriptRequest,
2027
ScriptListItem,
2128
ScriptRawResponse,
2229
ScriptResponse,
2330
)
31+
from auth import Session
2432
from config import Settings
25-
from dependencies import get_script_store, get_settings
33+
from dependencies import get_script_store, get_session, get_settings
2634
from fastapi import APIRouter, Depends, File, Form, HTTPException, Response, UploadFile
2735
from mail_stores import ScriptStore
2836
from sieve_transform import (
37+
generate_rule,
2938
generate_sieve,
3039
json_to_script,
3140
parse_sieve,
41+
rule_from_json,
3242
script_to_json,
3343
)
3444

@@ -90,6 +100,23 @@ def import_script(
90100
return {"ok": True, "name": name}
91101

92102

103+
@router.post("/preview", response_model=PreviewResponse)
104+
def preview_rule(req: PreviewRequest, _session: Session = Depends(get_session)):
105+
"""Render one Rule as the Sieve a save would write.
106+
107+
NO MAIL-SERVER DIAL. It depends on `get_session` and not on
108+
`get_script_store`, so it authenticates without opening a ManageSieve
109+
connection — this runs on every keystroke behind a debounce, and a
110+
connection per keystroke would be a self-inflicted denial of service
111+
against the user's own mail server.
112+
113+
It replaces `previewRule` in the SPA, which was a second implementation of
114+
`SieveGenerator` that had already diverged five ways. Declared BEFORE the
115+
`/{name}` routes so `preview` is read as a literal path segment.
116+
"""
117+
return {"sieve": generate_rule(rule_from_json(req.rule.model_dump()))}
118+
119+
93120
@router.put("/{name}", response_model=OkResponse, response_model_exclude_none=True)
94121
def save_script(name: str, req: SaveScriptRequest, store: ScriptStore = Depends(get_script_store)):
95122
"""Save script from JSON rules (generates Sieve)."""

backend/sieve_transform.py

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -486,12 +486,7 @@ def generate(self, script: SieveScript) -> str:
486486
# Generate in order — position in `entries` IS the order
487487
for entry in script.entries:
488488
if isinstance(entry, Rule):
489-
rule_text = self._generate_rule(entry)
490-
if not entry.enabled:
491-
rule_text = "\n".join(
492-
"## " + line if line.strip() else "##" for line in rule_text.split("\n")
493-
)
494-
parts.append(rule_text)
489+
parts.append(self.generate_entry(entry))
495490
parts.append("")
496491
else:
497492
if entry.comment:
@@ -501,6 +496,25 @@ def generate(self, script: SieveScript) -> str:
501496

502497
return "\n".join(parts).rstrip() + "\n"
503498

499+
def generate_entry(self, rule: Rule) -> str:
500+
"""The exact bytes one Rule contributes to a script.
501+
502+
Public because the preview endpoint calls it (areyousievious-8fg.17),
503+
and `generate` calls it too. That sharing is the point: the SPA used to
504+
carry `previewRule`, a SECOND implementation of this generator, and
505+
both modules said in a comment that the two "must agree" while nothing
506+
checked it. Five divergences shipped — dropped `negate`, no quote
507+
escaping, a disabled rule shown live, the missing `# --- name ---`
508+
line, and a conditionless Rule previewing as nothing while a save
509+
wrote `if anyof ( ) {`. There is now one implementation to diverge
510+
from.
511+
"""
512+
text = self._generate_rule(rule)
513+
if rule.enabled:
514+
return text
515+
# A disabled Rule is stored commented out.
516+
return "\n".join("## " + line if line.strip() else "##" for line in text.split("\n"))
517+
504518
def _compute_requires(self, script: SieveScript) -> list[str]:
505519
"""Compute required extensions from rules."""
506520
requires = set(script.requires)
@@ -704,3 +718,23 @@ def parse_sieve(text: str) -> SieveScript:
704718
def generate_sieve(script: SieveScript) -> str:
705719
"""Generate Sieve text from a SieveScript."""
706720
return SieveGenerator().generate(script)
721+
722+
723+
def rule_from_json(data: dict) -> Rule:
724+
"""Build a single Rule from its wire dict.
725+
726+
The rule-sized counterpart to `json_to_script`, for the preview endpoint —
727+
which has one Rule and no script to put it in.
728+
"""
729+
return _rule_from_json(data)
730+
731+
732+
def generate_rule(rule: Rule) -> str:
733+
"""The Sieve one Rule contributes to a script, byte for byte.
734+
735+
Goes through the same `SieveGenerator.generate_entry` a save does, so a
736+
preview cannot say one thing and a save write another. Note what this does
737+
NOT include: the `require [...]` line, which is a property of the whole
738+
script rather than of any one Rule.
739+
"""
740+
return SieveGenerator().generate_entry(rule)

0 commit comments

Comments
 (0)