Record a snapshot of the pages that never opened a browser

internal/distill/distill.go

@@ -529,7 +529,21 @@ func (d *Distiller) Distill(ctx context.Context, rawURL string) (*Result, error)
 		d.adoptServed(g, staticRes)
 		timing["total"] = time.Since(start)
 		d.progress(Progress{Stage: "done", Tier: escalate.TierFetch, Elapsed: time.Since(start)})
-		return &Result{Graph: g, Freshness: freshness, Decision: decision, Timing: timing}, nil
+		// The capture and the served bytes travel with the result here as they
+		// do on every other path.
+		//
+		// This one returned neither, and it is the path most pages take: a
+		// tier-0 answer is the commonest answer sieve gives. The visible effect
+		// was that -snapshot wrote a file containing nothing but a URL, so the
+		// pages whose bugs are cheapest to reproduce were exactly the ones that
+		// could not be attached to a report -- and replay refused the file with
+		// "contains no capture", which read like a corrupt snapshot rather than
+		// a snapshot that was never given anything to hold.
+		return &Result{
+			Graph: g, Freshness: freshness, Decision: decision, Timing: timing,
+			Capture: staticRes.Merged, StaticHTML: staticRes.RawHTML,
+			Status: int64(resp.Status),
+		}, nil
 	}
 
 	// --- Tiers 1-3 ----------------------------------------------------------

internal/snapshot/snapshot.go

@@ -36,6 +36,7 @@ import (
 
 	"github.com/qcoderx/sieve/internal/capture"
 	"github.com/qcoderx/sieve/internal/graph"
+	"github.com/qcoderx/sieve/internal/static"
 )
 
 // FormatVersion is bumped when the snapshot layout changes incompatibly.
@@ -69,7 +70,7 @@ type Snapshot struct {
 	// Redacted records that content was removed before writing. A snapshot from
 	// an authenticated session must never be attachable to a public bug report
 	// with the session's contents intact.
-	Redacted bool   `json:"redacted,omitempty"`
+	Redacted        bool   `json:"redacted,omitempty"`
 	RedactionReason string `json:"redaction_reason,omitempty"`
 }
 
@@ -163,8 +164,15 @@ func Read(path string) (*Snapshot, error) {
 		return nil, fmt.Errorf("%s was written by a newer sieve (format %d, this build understands %d)",
 			path, s.FormatVersion, FormatVersion)
 	}
-	if s.Merged == nil {
-		return nil, fmt.Errorf("%s contains no capture", path)
+	// A snapshot of a page answered without a browser has no capture, and that
+	// is not a broken file: tier 0 is the commonest answer sieve gives, and the
+	// graph stage runs over the served HTML there exactly as it runs over a
+	// rendered one. Refusing it meant a snapshot could not be taken of the very
+	// pages whose bugs are cheapest to reproduce -- the text welding and reading
+	// order defects fixed in this branch were all tier-0 defects, and none of
+	// them could have been attached to a report.
+	if s.Merged == nil && s.StaticHTML == "" {
+		return nil, fmt.Errorf("%s contains neither a capture nor the served HTML", path)
 	}
 	return &s, nil
 }
@@ -178,6 +186,20 @@ func Replay(s *Snapshot, in graph.Input) (*graph.Graph, error) {
 	in.RequestedURL = s.RequestedURL
 	in.FinalURL = s.FinalURL
 	in.Merged = s.Merged
+	// A page answered at tier 0 has no capture, only the bytes the server sent.
+	// Re-running the static extraction over those bytes reproduces the same
+	// input the live run built its graph from, which is what makes a tier-0 bug
+	// reproducible offline at all.
+	if in.Merged == nil && s.StaticHTML != "" {
+		res, err := static.Extract(s.FinalURL, strings.NewReader(s.StaticHTML), len(s.StaticHTML))
+		if err != nil {
+			return nil, fmt.Errorf("re-extract served HTML: %w", err)
+		}
+		in.Merged = res.Merged
+	}
+	if in.Merged == nil {
+		return nil, fmt.Errorf("snapshot contains neither a capture nor the served HTML")
+	}
 	if in.Notes == nil {
 		in.Notes = s.Notes
 	}

internal/snapshot/snapshot_test.go

@@ -0,0 +1,107 @@
+package snapshot_test
+
+import (
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/qcoderx/sieve/internal/graph"
+	"github.com/qcoderx/sieve/internal/snapshot"
+	"github.com/qcoderx/sieve/internal/static"
+)
+
+const servedPage = `<!doctype html>
+<html lang="en"><head><title>Halstead Instruments</title></head>
+<body>
+  <nav><a href="/">Home</a><a href="/about">About</a></nav>
+  <main>
+    <h1>Halstead Instruments</h1>
+    <p>We restore and calibrate laboratory balances, and hold the only
+       remaining stock of knife edges for the Oertling range.</p>
+    <h2>Workshop</h2>
+    <p>Calibration is traceable and certificates are issued the same week.
+       A restoration takes eight to twelve weeks depending on what we find
+       once the case is open.</p>
+  </main>
+</body></html>`
+
+// TestTierZeroSnapshotRoundTrips covers the pages a snapshot was least able to
+// describe: the ones answered without a browser.
+//
+// A tier-0 answer is the commonest answer sieve gives, and the result for that
+// path carried neither the capture nor the served bytes -- so -snapshot wrote a
+// file containing little more than a URL, and replay rejected it with
+// "contains no capture", which reads like a corrupt file rather than one that
+// was never given anything to hold. The effect was that the pages whose bugs
+// are cheapest to reproduce were exactly the ones no one could attach to a
+// report; every text-welding and reading-order defect fixed in this branch was
+// a tier-0 defect.
+func TestTierZeroSnapshotRoundTrips(t *testing.T) {
+	res, err := static.Extract("https://example.com/", strings.NewReader(servedPage), len(servedPage))
+	if err != nil {
+		t.Fatalf("extract: %v", err)
+	}
+
+	in := graph.Input{
+		RequestedURL: "https://example.com/",
+		FinalURL:     "https://example.com/",
+		Merged:       res.Merged,
+		Generator:    "sieve/test",
+	}
+	live, err := graph.Build(in)
+	if err != nil {
+		t.Fatalf("build: %v", err)
+	}
+	if len(live.Blocks) == 0 {
+		t.Fatal("fixture produced no blocks; the test would prove nothing")
+	}
+
+	// A snapshot as the tier-0 path records one: served HTML, no capture.
+	path := filepath.Join(t.TempDir(), "tier0.sieve")
+	if err := snapshot.Write(path, &snapshot.Snapshot{
+		RequestedURL: "https://example.com/",
+		FinalURL:     "https://example.com/",
+		StaticHTML:   servedPage,
+	}, snapshot.WriteOptions{IncludeHTML: true}); err != nil {
+		t.Fatalf("write: %v", err)
+	}
+
+	got, err := snapshot.Read(path)
+	if err != nil {
+		t.Fatalf("read: %v", err)
+	}
+	rebuilt, err := snapshot.Replay(got, graph.Input{Generator: "sieve/test"})
+	if err != nil {
+		t.Fatalf("replay: %v", err)
+	}
+
+	if len(rebuilt.Blocks) != len(live.Blocks) {
+		t.Fatalf("replay produced %d blocks, live produced %d",
+			len(rebuilt.Blocks), len(live.Blocks))
+	}
+	for i := range live.Blocks {
+		if live.Blocks[i].Text != rebuilt.Blocks[i].Text {
+			t.Errorf("block %d differs:\n live=%q\n repl=%q",
+				i, live.Blocks[i].Text, rebuilt.Blocks[i].Text)
+		}
+	}
+	if live.ContentHash != rebuilt.ContentHash {
+		t.Errorf("content hash differs: the graph stage is not a pure function "+
+			"of the snapshot\n live=%s\n repl=%s", live.ContentHash, rebuilt.ContentHash)
+	}
+}
+
+// TestSnapshotWithNothingInItIsRefused keeps the error honest: a file that
+// carries neither a capture nor the served bytes cannot be replayed, and should
+// say so rather than appearing to work.
+func TestSnapshotWithNothingInItIsRefused(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "empty.sieve")
+	if err := snapshot.Write(path, &snapshot.Snapshot{
+		RequestedURL: "https://example.com/",
+	}, snapshot.WriteOptions{}); err != nil {
+		t.Fatalf("write: %v", err)
+	}
+	if _, err := snapshot.Read(path); err == nil {
+		t.Error("a snapshot with no capture and no HTML was accepted")
+	}
+}