Merge pull request 'Progress reads only a design's head, so a long design never takes the progress view down (issue 1947)' (#623) from fix/1947-progress-bounded-read into main
This commit was merged in pull request #623.
This commit is contained in:
@@ -107,9 +107,12 @@ func newBusWorld(t *testing.T) *busWorld {
|
|||||||
return map[string]any{"folders": f, "documents": d}, err
|
return map[string]any{"folders": f, "documents": d}, err
|
||||||
})
|
})
|
||||||
answer("records.records_read", func(body json.RawMessage) (any, error) {
|
answer("records.records_read", func(body json.RawMessage) (any, error) {
|
||||||
var a struct{ Path string }
|
var a struct {
|
||||||
|
Path string
|
||||||
|
Head int
|
||||||
|
}
|
||||||
_ = json.Unmarshal(body, &a)
|
_ = json.Unmarshal(body, &a)
|
||||||
c, commit, err := records.Read(a.Path)
|
c, commit, err := records.ReadHead(a.Path, a.Head)
|
||||||
return map[string]any{"content": c, "commit": commit}, err
|
return map[string]any{"content": c, "commit": commit}, err
|
||||||
})
|
})
|
||||||
j := &judge{}
|
j := &judge{}
|
||||||
|
|||||||
@@ -24,8 +24,26 @@ type Design struct {
|
|||||||
Status string
|
Status string
|
||||||
Decisions []string
|
Decisions []string
|
||||||
Milestones []Milestone
|
Milestones []Milestone
|
||||||
|
// Unread says why the design's frontmatter and title could not be read, when they could not (novox/hq issue
|
||||||
|
// 1947); the progress answer carries it as a warning beside a tree built from the tracker's milestone records.
|
||||||
|
Unread string
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// unreadWarnings says each design whose head was not read, so none drops out of an answer silently.
|
||||||
|
func unreadWarnings(ds []*Design) []string {
|
||||||
|
var out []string
|
||||||
|
for _, d := range ds {
|
||||||
|
if d.Unread != "" {
|
||||||
|
out = append(out, d.Unread)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
|
// DesignHeadBytes is how much of a design is read: its frontmatter and its title lead it, and its milestones are the
|
||||||
|
// tracker's records, so the rest is never needed.
|
||||||
|
const DesignHeadBytes = 16 << 10
|
||||||
|
|
||||||
// Milestone is one milestone of a design, as the issue tracker keeps it.
|
// Milestone is one milestone of a design, as the issue tracker keeps it.
|
||||||
type Milestone struct {
|
type Milestone struct {
|
||||||
ID string
|
ID string
|
||||||
@@ -51,12 +69,13 @@ func parseDesign(path, content string) *Design {
|
|||||||
d.Number, _ = strconv.Atoi(m[1])
|
d.Number, _ = strconv.Atoi(m[1])
|
||||||
}
|
}
|
||||||
lines := strings.Split(content, "\n")
|
lines := strings.Split(content, "\n")
|
||||||
body := 0
|
body, closed := 0, false
|
||||||
if len(lines) > 0 && strings.TrimSpace(lines[0]) == "---" {
|
if len(lines) > 0 && strings.TrimSpace(lines[0]) == "---" {
|
||||||
for i := 1; i < len(lines); i++ {
|
for i := 1; i < len(lines); i++ {
|
||||||
l := lines[i]
|
l := lines[i]
|
||||||
if strings.TrimSpace(l) == "---" {
|
if strings.TrimSpace(l) == "---" {
|
||||||
body = i + 1
|
body = i + 1
|
||||||
|
closed = true
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
if v, ok := strings.CutPrefix(l, "status:"); ok {
|
if v, ok := strings.CutPrefix(l, "status:"); ok {
|
||||||
@@ -67,6 +86,9 @@ func parseDesign(path, content string) *Design {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if len(lines) > 0 && strings.TrimSpace(lines[0]) == "---" && !closed {
|
||||||
|
d.Unread = fmt.Sprintf("the frontmatter of %s does not close in the part read: its state and decisions are not known here", path)
|
||||||
|
}
|
||||||
for i := body; i < len(lines) && d.Title == ""; i++ {
|
for i := body; i < len(lines) && d.Title == ""; i++ {
|
||||||
if m := titleLine.FindStringSubmatch(strings.TrimSpace(lines[i])); m != nil {
|
if m := titleLine.FindStringSubmatch(strings.TrimSpace(lines[i])); m != nil {
|
||||||
d.Title = strings.TrimSpace(m[1])
|
d.Title = strings.TrimSpace(m[1])
|
||||||
@@ -151,12 +173,22 @@ func (k *KB) designs(all bool, number int) ([]*Design, string, error) {
|
|||||||
}
|
}
|
||||||
d := c.byPath[p]
|
d := c.byPath[p]
|
||||||
if d == nil {
|
if d == nil {
|
||||||
content, _, err := k.Records.Read(p)
|
var content string
|
||||||
|
var err error
|
||||||
|
if h, ok := k.Records.(HeadReader); ok {
|
||||||
|
content, _, err = h.ReadHead(p, DesignHeadBytes)
|
||||||
|
} else {
|
||||||
|
content, _, err = k.Records.Read(p)
|
||||||
|
}
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, "", fmt.Errorf("the records module does not read %s: %v", p, err)
|
d = parseDesign(p, "")
|
||||||
|
d.Unread = fmt.Sprintf("the records module does not read %s (%v): its title and state are not known here", p, err)
|
||||||
|
} else {
|
||||||
|
d = parseDesign(p, content)
|
||||||
|
}
|
||||||
|
if d.Unread == "" {
|
||||||
|
c.byPath[p] = d
|
||||||
}
|
}
|
||||||
d = parseDesign(p, content)
|
|
||||||
c.byPath[p] = d
|
|
||||||
k.progReads++
|
k.progReads++
|
||||||
}
|
}
|
||||||
out = append(out, d)
|
out = append(out, d)
|
||||||
|
|||||||
@@ -22,6 +22,12 @@ type Records interface {
|
|||||||
Read(path string) (content, commit string, err error)
|
Read(path string) (content, commit string, err error)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// HeadReader is Records that can read only the first part of a document: a caller that needs a frontmatter or a
|
||||||
|
// title reads a long document through it, bounded, never whole (novox/hq issue 1947).
|
||||||
|
type HeadReader interface {
|
||||||
|
ReadHead(path string, bytes int) (content, commit string, err error)
|
||||||
|
}
|
||||||
|
|
||||||
// busRecords is Records over the bus: the records module's tools, called as this module.
|
// busRecords is Records over the bus: the records module's tools, called as this module.
|
||||||
type busRecords struct{}
|
type busRecords struct{}
|
||||||
|
|
||||||
@@ -58,8 +64,18 @@ func (busRecords) List(folder string) ([]string, []string, error) {
|
|||||||
return f.Folders, f.Documents, nil
|
return f.Folders, f.Documents, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func (busRecords) Read(path string) (string, string, error) {
|
func (busRecords) Read(path string) (string, string, error) { return busRead(path, 0) }
|
||||||
raw, err := stdio.Ask("records.records_read", map[string]any{"path": path})
|
|
||||||
|
func (busRecords) ReadHead(path string, bytes int) (string, string, error) {
|
||||||
|
return busRead(path, bytes)
|
||||||
|
}
|
||||||
|
|
||||||
|
func busRead(path string, head int) (string, string, error) {
|
||||||
|
args := map[string]any{"path": path}
|
||||||
|
if head > 0 {
|
||||||
|
args["head"] = head
|
||||||
|
}
|
||||||
|
raw, err := stdio.Ask("records.records_read", args)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", "", err
|
return "", "", err
|
||||||
}
|
}
|
||||||
@@ -118,11 +134,19 @@ func (d dirRecords) List(folder string) ([]string, []string, error) {
|
|||||||
return folders, docs, nil
|
return folders, docs, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func (d dirRecords) Read(path string) (string, string, error) {
|
func (d dirRecords) Read(path string) (string, string, error) { return d.ReadHead(path, 0) }
|
||||||
|
|
||||||
|
func (d dirRecords) ReadHead(path string, head int) (string, string, error) {
|
||||||
raw, err := os.ReadFile(filepath.Join(d.dir, filepath.FromSlash(path)))
|
raw, err := os.ReadFile(filepath.Join(d.dir, filepath.FromSlash(path)))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", "", err
|
return "", "", err
|
||||||
}
|
}
|
||||||
|
if head > 0 && head < len(raw) {
|
||||||
|
raw = raw[:head]
|
||||||
|
if i := strings.LastIndexByte(string(raw), '\n'); i > 0 {
|
||||||
|
raw = raw[:i+1]
|
||||||
|
}
|
||||||
|
}
|
||||||
commit, _ := d.Standing()
|
commit, _ := d.Standing()
|
||||||
return string(raw), commit, nil
|
return string(raw), commit, nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -329,6 +329,12 @@ func (k *KB) progressDesign(p *Progress, number int, milestone string) error {
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
d := ds[0]
|
d := ds[0]
|
||||||
|
if d.Unread != "" {
|
||||||
|
p.Warnings = append(p.Warnings, d.Unread)
|
||||||
|
if d.Title == "" {
|
||||||
|
d.Title = fmt.Sprintf("design %d", d.Number)
|
||||||
|
}
|
||||||
|
}
|
||||||
if len(d.Milestones) == 0 {
|
if len(d.Milestones) == 0 {
|
||||||
return fmt.Errorf("design %d has no milestone records in the issue tracker: seed them from its table with "+
|
return fmt.Errorf("design %d has no milestone records in the issue tracker: seed them from its table with "+
|
||||||
"issue-tracker.import milestones %d", number, number)
|
"issue-tracker.import milestones %d", number, number)
|
||||||
@@ -387,6 +393,7 @@ func (k *KB) progressMesh(p *Progress) error {
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
p.Source = fmt.Sprintf("the issue tracker's milestone records, and the designs in %s at hq %s", DesignsFolder, short(commit))
|
p.Source = fmt.Sprintf("the issue tracker's milestone records, and the designs in %s at hq %s", DesignsFolder, short(commit))
|
||||||
|
p.Warnings = append(p.Warnings, unreadWarnings(ds)...)
|
||||||
var with []*Design
|
var with []*Design
|
||||||
var without []string
|
var without []string
|
||||||
for _, d := range ds {
|
for _, d := range ds {
|
||||||
@@ -514,6 +521,7 @@ func (k *KB) progressRecord(p *Progress, kind string, n int) error {
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
p.Source = fmt.Sprintf("the knowledge base, the designs at hq %s and the issue tracker's links", short(commit))
|
p.Source = fmt.Sprintf("the knowledge base, the designs at hq %s and the issue tracker's links", short(commit))
|
||||||
|
p.Warnings = append(p.Warnings, unreadWarnings(ds)...)
|
||||||
var designs []string
|
var designs []string
|
||||||
for _, d := range ds {
|
for _, d := range ds {
|
||||||
for _, dn := range d.Decisions {
|
for _, dn := range d.Decisions {
|
||||||
|
|||||||
@@ -42,6 +42,15 @@ func (w withDesigns) Read(path string) (string, string, error) {
|
|||||||
return c, commit, err
|
return c, commit, err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (w withDesigns) ReadHead(path string, n int) (string, string, error) {
|
||||||
|
if c, commit, err := w.base.ReadHead(path, n); err == nil {
|
||||||
|
return c, commit, nil
|
||||||
|
}
|
||||||
|
c, _, err := w.designs.ReadHead(path, n)
|
||||||
|
commit, _ := w.base.Standing()
|
||||||
|
return c, commit, err
|
||||||
|
}
|
||||||
|
|
||||||
// seatsWorld plays the issue tracker and mesh-delivery from the captured answers, and counts what it is asked.
|
// seatsWorld plays the issue tracker and mesh-delivery from the captured answers, and counts what it is asked.
|
||||||
type seatsWorld struct {
|
type seatsWorld struct {
|
||||||
mu sync.Mutex
|
mu sync.Mutex
|
||||||
@@ -142,6 +151,19 @@ func (c *countingRecords) Read(p string) (string, string, error) {
|
|||||||
return content, c.commit, err
|
return content, c.commit, err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (c *countingRecords) ReadHead(p string, n int) (string, string, error) {
|
||||||
|
c.reads++
|
||||||
|
content, _, err := c.dirRecords.ReadHead(p, n)
|
||||||
|
return content, c.commit, err
|
||||||
|
}
|
||||||
|
|
||||||
|
// brokenRead is Records whose reads fail as the records module's do for a document longer than it answers whole.
|
||||||
|
type brokenRead struct{ dirRecords }
|
||||||
|
|
||||||
|
func (brokenRead) ReadHead(p string, n int) (string, string, error) {
|
||||||
|
return "", "", fmt.Errorf("%s is longer than the records module answers whole", p)
|
||||||
|
}
|
||||||
|
|
||||||
// 1947/AC2, as issue 1781 changed it: a design's title, state and decisions are hq's; its milestones are the issue
|
// 1947/AC2, as issue 1781 changed it: a design's title, state and decisions are hq's; its milestones are the issue
|
||||||
// tracker's records, ordered as its table, an id found with spaces or hyphens, and a closed one holding no issue done.
|
// tracker's records, ordered as its table, an id found with spaces or hyphens, and a closed one holding no issue done.
|
||||||
func TestADesignsMilestonesAreTheTrackersRecords(t *testing.T) {
|
func TestADesignsMilestonesAreTheTrackersRecords(t *testing.T) {
|
||||||
@@ -316,6 +338,18 @@ func TestProgressOfADesignMinimalAndDetailed(t *testing.T) {
|
|||||||
if big, _ := k.Progress(ProgressAsk{Of: "design 47", Bound: 8192}); big.Cut != "" || big.Tree != p.Tree {
|
if big, _ := k.Progress(ProgressAsk{Of: "design 47", Bound: 8192}); big.Cut != "" || big.Tree != p.Tree {
|
||||||
t.Errorf("a minimal answer at bound 8192 is cut: %q", big.Cut)
|
t.Errorf("a minimal answer at bound 8192 is cut: %q", big.Cut)
|
||||||
}
|
}
|
||||||
|
// A design whose head the records module cannot read is said in a warning beside the tracker-built tree, never a failure.
|
||||||
|
kb := &KB{Records: brokenRead{progressRecords()}, Seats: newSeatsWorld(t).ask, Now: k.Now}
|
||||||
|
if bp, err := kb.Progress(ProgressAsk{Of: "design 47"}); err != nil || !strings.Contains(strings.Join(bp.Warnings, "\n"), "does not read") ||
|
||||||
|
!strings.Contains(bp.Tree, "design 47") || !strings.Contains(bp.Tree, "R6") {
|
||||||
|
t.Fatalf("an unreadable design: %v %+v", err, bp)
|
||||||
|
}
|
||||||
|
if mp, err := kb.Progress(ProgressAsk{Of: "mesh"}); err != nil || !strings.Contains(strings.Join(mp.Warnings, "\n"), "does not read") {
|
||||||
|
t.Fatalf("the mesh level drops an unread design silently: %v %v", err, mp.Warnings)
|
||||||
|
}
|
||||||
|
if d := parseDesign(DesignsFolder+"/47-x.md", "---\nstatus: in-progress\n"); d.Unread == "" {
|
||||||
|
t.Errorf("a frontmatter cut before its close is not said")
|
||||||
|
}
|
||||||
lines := strings.Split(p.Tree, "\n")
|
lines := strings.Split(p.Tree, "\n")
|
||||||
if !strings.HasPrefix(lines[0], "✗ design 47 · Delivery: from a commit to delivered — ") {
|
if !strings.HasPrefix(lines[0], "✗ design 47 · Delivery: from a commit to delivered — ") {
|
||||||
t.Fatalf("first line %q", lines[0])
|
t.Fatalf("first line %q", lines[0])
|
||||||
|
|||||||
@@ -52,9 +52,16 @@ func tools(r *Records) []stdio.Tool {
|
|||||||
return r.Search(strArg(a, "query"), limit)
|
return r.Search(strArg(a, "query"), limit)
|
||||||
}},
|
}},
|
||||||
{Name: "records_read",
|
{Name: "records_read",
|
||||||
Description: "One document, whole, by its path in the repository — a decision record, a design document, an issue report.",
|
Description: "One document by its path (whole, or its first part with head) in the repository — a decision record, a design document, an issue report.",
|
||||||
Input: map[string]any{"path": str("the document's path, e.g. 02-DECISIONS/0025-....md")},
|
Input: map[string]any{"path": str("the document's path, e.g. 02-DECISIONS/0025-....md"),
|
||||||
Run: func(a map[string]any) (any, error) { return r.Read(strArg(a, "path")) }},
|
"head": map[string]any{"type": "number", "description": "only the first this many bytes, cut at a line end (optional); the answer says partial"}},
|
||||||
|
Run: func(a map[string]any) (any, error) {
|
||||||
|
head := 0
|
||||||
|
if v, ok := a["head"].(float64); ok {
|
||||||
|
head = int(v)
|
||||||
|
}
|
||||||
|
return r.ReadHead(strArg(a, "path"), head)
|
||||||
|
}},
|
||||||
{Name: "records_list",
|
{Name: "records_list",
|
||||||
Description: "What a folder of the repository holds: its sub-folders and its documents. The root when no folder is named.",
|
Description: "What a folder of the repository holds: its sub-folders and its documents. The root when no folder is named.",
|
||||||
Input: map[string]any{"folder": str("a folder inside the repository (optional)")},
|
Input: map[string]any{"folder": str("a folder inside the repository (optional)")},
|
||||||
|
|||||||
@@ -322,12 +322,18 @@ type Document struct {
|
|||||||
Path string `json:"path"`
|
Path string `json:"path"`
|
||||||
Content string `json:"content"`
|
Content string `json:"content"`
|
||||||
Truncated bool `json:"truncated"`
|
Truncated bool `json:"truncated"`
|
||||||
Commit string `json:"commit"`
|
// Partial is true when only the first part the caller asked for (head) is answered; Truncated stays the answer's own cap.
|
||||||
|
Partial bool `json:"partial,omitempty"`
|
||||||
|
Commit string `json:"commit"`
|
||||||
}
|
}
|
||||||
|
|
||||||
// Read is one document, whole, or its first part with a note when it is very long. The path stays inside
|
// Read is one document, whole, or its first part with a note when it is very long. The path stays inside
|
||||||
// the checkout: `..` and absolute paths are refused, not resolved.
|
// the checkout: `..` and absolute paths are refused, not resolved.
|
||||||
func (r *Records) Read(p string) (Document, error) {
|
func (r *Records) Read(p string) (Document, error) { return r.ReadHead(p, 0) }
|
||||||
|
|
||||||
|
// ReadHead is Read, or with head above zero only the first head bytes of the document, cut at a line end, so a
|
||||||
|
// caller that needs a frontmatter or a title reads a long document without its whole length (novox/hq issue 1947).
|
||||||
|
func (r *Records) ReadHead(p string, head int) (Document, error) {
|
||||||
clean, ok := inside(p)
|
clean, ok := inside(p)
|
||||||
if !ok || clean == "" {
|
if !ok || clean == "" {
|
||||||
return Document{}, fmt.Errorf("%q is not a path inside the repository", p)
|
return Document{}, fmt.Errorf("%q is not a path inside the repository", p)
|
||||||
@@ -337,6 +343,15 @@ func (r *Records) Read(p string) (Document, error) {
|
|||||||
return Document{}, fmt.Errorf("the repository holds no %s — `records_list` says what it holds", clean)
|
return Document{}, fmt.Errorf("the repository holds no %s — `records_list` says what it holds", clean)
|
||||||
}
|
}
|
||||||
d := Document{Path: clean, Content: string(raw), Commit: r.Standing().Commit}
|
d := Document{Path: clean, Content: string(raw), Commit: r.Standing().Commit}
|
||||||
|
head = min(head, MostBytes)
|
||||||
|
if head > 0 && head < len(d.Content) {
|
||||||
|
cut := d.Content[:head]
|
||||||
|
if i := strings.LastIndex(cut, "\n"); i > 0 {
|
||||||
|
cut = cut[:i+1]
|
||||||
|
}
|
||||||
|
d.Content, d.Partial = cut, true
|
||||||
|
return d, nil
|
||||||
|
}
|
||||||
if len(d.Content) > MostBytes {
|
if len(d.Content) > MostBytes {
|
||||||
d.Content = d.Content[:MostBytes] + "\n\n[… truncated; the document is longer than this answer carries]"
|
d.Content = d.Content[:MostBytes] + "\n\n[… truncated; the document is longer than this answer carries]"
|
||||||
d.Truncated = true
|
d.Truncated = true
|
||||||
|
|||||||
@@ -70,6 +70,10 @@ func TestAPhraseInOneDesignDocumentComesBackFromWhereItIsWritten(t *testing.T) {
|
|||||||
if err != nil || !strings.Contains(doc.Content, "We decided the thing") || doc.Truncated {
|
if err != nil || !strings.Contains(doc.Content, "We decided the thing") || doc.Truncated {
|
||||||
t.Fatalf("%+v %v", doc, err)
|
t.Fatalf("%+v %v", doc, err)
|
||||||
}
|
}
|
||||||
|
if head, err := r.ReadHead("02-DECISIONS/0001-a-decision.md", 30); err != nil || !head.Partial || head.Truncated || len(head.Content) > 30 ||
|
||||||
|
!strings.HasSuffix(head.Content, "\n") || !strings.HasPrefix(doc.Content, head.Content) {
|
||||||
|
t.Fatalf("a head read: %+v %v", head, err)
|
||||||
|
}
|
||||||
root, _ := r.List("")
|
root, _ := r.List("")
|
||||||
if strings.Join(root.Folders, ",") != "02-DECISIONS,03-DESIGN" || strings.Join(root.Documents, ",") != "README.md" {
|
if strings.Join(root.Folders, ",") != "02-DECISIONS,03-DESIGN" || strings.Join(root.Documents, ",") != "README.md" {
|
||||||
t.Fatalf("%+v", root)
|
t.Fatalf("%+v", root)
|
||||||
|
|||||||
Reference in New Issue
Block a user