From bbaaa1f7b2e833698b0db403de50fc6f160c2bae Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Thu, 13 Aug 2026 11:10:39 +0300 Subject: [PATCH] web: the milestones page says when its rollup is partial MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MilestoneView has carried the clip since ebbd30f and list_milestones has reported it since a419499; milestones.html rendered nothing, so the page was the last surface presenting a partial rollup as whole-tracker arithmetic. Every number on it is a rollup — "1/4 done", "1995 of 2000 issues carry no milestone label" — and arithmetic over the rows that were read renders exactly like arithmetic over the tracker. The notice is the detail pane's, word for word and class for class: it is the same fact about the same read, and a second phrasing for it would be a second style. It sits above the rollup and outside the milestones/no-milestones branch, because "No milestones." over a clipped read is the reading most easily mistaken for a fact about the tracker. A milestone can lose members here rather than merely undercount them — membership comes from labels — so the fixture makes that concrete: m1 has five members, one of them past the cap, and the page counts four while saying why. --- web/milestones_test.go | 135 ++++++++++++++++++++++++++++++++++ web/templates/milestones.html | 14 ++++ 2 files changed, 149 insertions(+) diff --git a/web/milestones_test.go b/web/milestones_test.go index 6c8b968874843d289242067ebc9e4613ead1bdc1..363b0e8bce17c83f1fc01b9a5dc1c7031ad5ab86 100644 --- a/web/milestones_test.go +++ b/web/milestones_test.go @@ -1,10 +1,12 @@ package web import ( + "fmt" "net/http" "strings" "testing" + "sourcecraft.dev/bigbes/sr-ht-dolt/beads" "sourcecraft.dev/bigbes/sr-ht-dolt/browse" "sourcecraft.dev/bigbes/sr-ht-dolt/core" ) @@ -65,6 +67,54 @@ func milestoneFixture() *fakeSession { } } +// milestoneClippedFixture is that same tracker grown past the cap: the six rows +// above, filler up to beads.Max, and five more — among them i-tail, which +// carries milestone:m1 and sits in the tail no read here reaches. The read hands +// over the first beads.Max rows and a Total saying 2005 exist. +// +// So m1 rolls up four of its five members, and every number on the page is +// arithmetic over a partial read while looking exactly like arithmetic over the +// tracker. The rows are built whole and then clipped, rather than a short page +// given a large Total by hand: the missing member has to be genuinely absent +// from what the projection reads, or the test asserts the notice while never +// producing the situation the notice is about. +func milestoneClippedFixture() *fakeSession { + sess := milestoneFixture() + issues := sess.rowsByTable["issues"] + rows := append([][]string{}, issues.Rows...) + for i := len(rows); i < beads.Max; i++ { + rows = append(rows, []string{fmt.Sprintf("i-%04d", i), fmt.Sprintf("Filler %d", i), "open", "1", "task", ""}) + } + rows = append(rows, []string{"i-tail", "Tail task", "open", "1", "task", "alice"}) + for i := len(rows); i < beads.Max+5; i++ { + rows = append(rows, []string{fmt.Sprintf("i-%04d", i), fmt.Sprintf("Filler %d", i), "open", "1", "task", ""}) + } + sess.rowsByTable["issues"] = clipPage( + &browse.RowPage{Columns: issues.Columns, Rows: rows, Total: len(rows)}, beads.Max) + + labels := sess.rowsByTable["labels"] + labels.Rows = append(labels.Rows, []string{"i-tail", "milestone:m1"}) + labels.Total = len(labels.Rows) + return sess +} + +// milestoneClippedLabelsFixture leaves every issue readable and clips the table +// that decides milestone membership instead: the six real label rows lead, so +// the rollup still renders, and beads.Max+3 rows exist of which the read returns +// beads.Max. Nothing about the issue count is wrong here, which is why the +// notice's second sentence has to be conditional. +func milestoneClippedLabelsFixture() *fakeSession { + sess := milestoneFixture() + labels := sess.rowsByTable["labels"] + rows := append([][]string{}, labels.Rows...) + for i := len(rows); i < beads.Max+3; i++ { + rows = append(rows, []string{"i-d", fmt.Sprintf("label-%04d", i)}) + } + sess.rowsByTable["labels"] = clipPage( + &browse.RowPage{Columns: labels.Columns, Rows: rows, Total: len(rows)}, beads.Max) + return sess +} + func TestMilestonesApplies(t *testing.T) { // The milestone tab appears exactly where the beads tab does. if (&milestonesView{}).Applies(beadsTables()) != (&beadsView{}).Applies(beadsTables()) { @@ -103,4 +153,89 @@ func TestMilestonesRender(t *testing.T) { t.Errorf("milestones render missing %q", want) } } + // Nothing was clipped here, so the page makes no claim about a partial read. + if strings.Contains(body, "This read was clipped") { + t.Errorf("a complete read must not be dressed up as a clipped one; body=%s", body) + } +} + +// --- a read that was clipped ------------------------------------------------- + +// Every count on this page is a rollup: "1/4 done", "N of M carry no milestone +// label". Over a clipped read they are arithmetic over the rows that were read +// and they render identically to arithmetic over the tracker, so the page has to +// say which one it is — in the detail pane's words, for the same fact. +func TestMilestonesReportAClippedRead(t *testing.T) { + h := newHarness(t) + h.store.add(&core.Repo{Name: "db", OwnerID: 1, OwnerName: "alice", Path: "/d", Visibility: core.VisibilityPublic}) + h.browse.sess = milestoneClippedFixture() + setViews(t, h, &beadsView{}, &milestonesView{}) + + rec := h.do("GET", "/~alice/db/view/milestones", nil, nil) + if rec.Code != http.StatusOK { + t.Fatalf("milestones view: got %d; body=%s", rec.Code, rec.Body.String()) + } + body := rec.Body.String() + if !strings.Contains(body, "This read was clipped") { + t.Errorf("the rollup is over a partial read and the page says nothing; body=%s", body) + } + // The number is the tracker's true size, which is what makes the line + // checkable rather than a bare warning. + if !strings.Contains(body, "The tracker holds 2005 issues and only the first of them were read.") { + t.Errorf("the page does not name the tracker's true size; body=%s", body) + } + // And the arithmetic the notice is about: m1 has five members and the page + // counts four, because the fifth is past the cap. + if !strings.Contains(body, "1/4 done") { + t.Errorf("m1's rollup is the members that were read; body=%s", body) + } + if strings.Contains(body, "Tail task") { + t.Errorf("the tail member cannot be on the page; body=%s", body) + } +} + +// A clip in labels — the table that decides membership — with every issue read. +// The notice appears; its second sentence does not, because the issue count is +// not the thing that went short. +func TestMilestonesReportAClippedLabelsRead(t *testing.T) { + h := newHarness(t) + h.store.add(&core.Repo{Name: "db", OwnerID: 1, OwnerName: "alice", Path: "/d", Visibility: core.VisibilityPublic}) + h.browse.sess = milestoneClippedLabelsFixture() + setViews(t, h, &beadsView{}, &milestonesView{}) + + rec := h.do("GET", "/~alice/db/view/milestones", nil, nil) + if rec.Code != http.StatusOK { + t.Fatalf("milestones view: got %d; body=%s", rec.Code, rec.Body.String()) + } + body := rec.Body.String() + if !strings.Contains(body, "This read was clipped") { + t.Errorf("membership itself was read in part and the page says nothing; body=%s", body) + } + if strings.Contains(body, "The tracker holds") { + t.Errorf("every issue was read, so the page must not claim otherwise; body=%s", body) + } +} + +// The page with no milestones at all still carries the notice: an empty rollup +// over a clipped read is the one most easily mistaken for a fact about the +// tracker. +func TestMilestonesReportAClippedReadWithNoMilestones(t *testing.T) { + h := newHarness(t) + h.store.add(&core.Repo{Name: "db", OwnerID: 1, OwnerName: "alice", Path: "/d", Visibility: core.VisibilityPublic}) + sess := milestoneClippedFixture() + sess.rowsByTable["labels"] = &browse.RowPage{Columns: []string{"issue_id", "label"}} + h.browse.sess = sess + setViews(t, h, &beadsView{}, &milestonesView{}) + + rec := h.do("GET", "/~alice/db/view/milestones", nil, nil) + if rec.Code != http.StatusOK { + t.Fatalf("milestones view: got %d; body=%s", rec.Code, rec.Body.String()) + } + body := rec.Body.String() + if !strings.Contains(body, "No milestones.") { + t.Fatalf("the fixture is meant to produce an empty rollup; body=%s", body) + } + if !strings.Contains(body, "This read was clipped") { + t.Errorf("\"no milestones\" over a clipped read is not an answer about the tracker; body=%s", body) + } } diff --git a/web/templates/milestones.html b/web/templates/milestones.html index bc04b3e137ea3b7703ffa529cce1d8522c182948..36978e92b2520d8d2a73ad5a620328e2a5d1ed7a 100644 --- a/web/templates/milestones.html +++ b/web/templates/milestones.html @@ -58,6 +58,20 @@ {{template "beadsHead" (dict "Repo" .Repo "Ref" .Ref "Head" .Head)}} {{template "viewtabs" (dict "Repo" .Repo "Views" .Views "Current" "milestones" "Ref" .Ref)}} +{{/* Every number below this line is a rollup — "1/4 done", "N of M carry no + milestone label" — and arithmetic over the rows that were read renders + exactly like arithmetic over the tracker. A milestone can lose members + outright here, not merely undercount them: membership comes from labels, and + a clip in either table takes an issue off its milestone silently. The + wording and the markup are the detail pane's, because it is the same fact + about the same read; a second phrasing for it would be a second style. It + sits above the rollup rather than under it, and outside the + milestones/no-milestones branch, because "No milestones." over a clipped + read is the reading most easily mistaken for a fact about the tracker. */}} +{{if .Data.Truncated}} +
This read was clipped, so what is below may be short.{{if .Data.IssuesClipped}} The tracker holds {{.Data.ShownOf}} issues and only the first of them were read.{{end}}
+{{end}} + {{if .Data.Milestones}} {{range .Data.Milestones}}