mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-27 06:47:07 +00:00
fix(server): item context fills its budget past a part that does not fit, orders conventions by relevance, and widens implement to commit and PR rules (PLAN-3598) (#2101)
The lead's dogfood of `pad item context TASK-3613 --intent implement` found three selection defects before slice (d) switches the skill over: - The budget stopped at the first part that did not fit: 17k of 40k used, with 50+ parts dropped behind one 32 KB convention. A part that does not fit is now skipped whole and filling continues; the parent chain alone still stops at its first miss, since a gap in it would misstate lineage. - Conventions were ordered by priority, then by ref STRING (CONVE-1287 before CONVE-13), so unrelated always-on rules spent the budget first. They are now ordered by relevance: the intent's own triggers, then rules scoped to the caller's role, then global always-on ones; within a tier priority, then the smaller body, then the ref's number. Playbooks put the intent's triggers ahead of always-on ones, by ref number. - implement now also pulls on-commit and on-pr-create where the workspace declares them, so CONVE-735 (the mandatory review, on-pr-create) is in an implement brief. Every omitted entry now names its part, ref and title (decisions: the item ref and the question key), where about 40 read ref: null. Tests: keep filling (no omitted part fits the leftover), a 32 KB must rule does not crowd out small ones, relevance order, and an on-pr-create rule in an implement brief with a commit-brief control. One mutant per fix (restore stop-at-first, the ref-string order, no widening, untitled omits) each fails the tests aimed at it. Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS
This commit is contained in:
@@ -94,6 +94,7 @@ type itemContextDeps struct {
|
|||||||
type itemContextOmitted struct {
|
type itemContextOmitted struct {
|
||||||
Part string `json:"part"`
|
Part string `json:"part"`
|
||||||
Ref string `json:"ref,omitempty"`
|
Ref string `json:"ref,omitempty"`
|
||||||
|
Title string `json:"title,omitempty"`
|
||||||
Seq int64 `json:"seq,omitempty"`
|
Seq int64 `json:"seq,omitempty"`
|
||||||
Size int `json:"size"`
|
Size int `json:"size"`
|
||||||
Reason string `json:"reason"`
|
Reason string `json:"reason"`
|
||||||
@@ -178,7 +179,7 @@ func (s *Server) handleGetItemContext(w http.ResponseWriter, r *http.Request) {
|
|||||||
if intent != "" && convColl == nil {
|
if intent != "" && convColl == nil {
|
||||||
triggers = append(triggers, "on-"+strings.TrimPrefix(intent, "on-"))
|
triggers = append(triggers, "on-"+strings.TrimPrefix(intent, "on-"))
|
||||||
} else if intent != "" {
|
} else if intent != "" {
|
||||||
trig, valid, terr := s.resolveContextIntent(convColl, intent)
|
trig, also, valid, terr := s.resolveContextIntent(convColl, intent)
|
||||||
if terr != nil {
|
if terr != nil {
|
||||||
writeInternalError(w, terr)
|
writeInternalError(w, terr)
|
||||||
return
|
return
|
||||||
@@ -192,6 +193,7 @@ func (s *Server) handleGetItemContext(w http.ResponseWriter, r *http.Request) {
|
|||||||
if trig != "always" {
|
if trig != "always" {
|
||||||
triggers = append(triggers, trig)
|
triggers = append(triggers, trig)
|
||||||
}
|
}
|
||||||
|
triggers = append(triggers, also...)
|
||||||
}
|
}
|
||||||
|
|
||||||
role := strings.TrimSpace(q.Get("role"))
|
role := strings.TrimSpace(q.Get("role"))
|
||||||
@@ -255,21 +257,23 @@ func (s *Server) handleGetItemContext(w http.ResponseWriter, r *http.Request) {
|
|||||||
Detail: item.Ref + " is open but its parent " + a.Ref + " is closed"})
|
Detail: item.Ref + " is open but its parent " + a.Ref + " is closed"})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if !b.take("parent", a.Ref, a.Seq, p) {
|
// The chain stops at its first miss: an ancestor shown above a
|
||||||
|
// missing one would misstate the lineage.
|
||||||
|
if !b.take("parent", a.Ref, a.Title, a.Seq, p) {
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
resp.Parents = append(resp.Parents, p)
|
resp.Parents = append(resp.Parents, p)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Conventions whose trigger matches, deduplicated, must → should → nice,
|
// Conventions whose trigger matches, deduplicated, in relevance order
|
||||||
// then by ref; role-scoped ones only for the named role when one is given.
|
// (contextConventions); role-scoped ones only for the named role.
|
||||||
convs, err := s.contextConventions(r, workspaceID, convColl, triggers, role)
|
convs, err := s.contextConventions(r, workspaceID, convColl, triggers, role)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
writeInternalError(w, err)
|
writeInternalError(w, err)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
for _, c := range convs {
|
for _, c := range convs {
|
||||||
if !b.take("convention", c.Ref, c.Seq, c) {
|
if !b.take("convention", c.Ref, c.Title, c.Seq, c) {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
resp.Conventions = append(resp.Conventions, c)
|
resp.Conventions = append(resp.Conventions, c)
|
||||||
@@ -283,7 +287,7 @@ func (s *Server) handleGetItemContext(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
for _, p := range pbs {
|
for _, p := range pbs {
|
||||||
if !b.take("playbook", p.Ref, 0, p) {
|
if !b.take("playbook", p.Ref, p.Title, 0, p) {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
resp.Playbooks = append(resp.Playbooks, p)
|
resp.Playbooks = append(resp.Playbooks, p)
|
||||||
@@ -330,7 +334,7 @@ func (s *Server) handleGetItemContext(w http.ResponseWriter, r *http.Request) {
|
|||||||
others = append(others, e)
|
others = append(others, e)
|
||||||
}
|
}
|
||||||
for i, d := range deps {
|
for i, d := range deps {
|
||||||
if !b.take("dependency", d.Ref, 0, d) {
|
if !b.take("dependency", d.Ref, d.Title, 0, d) {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
if depKind[i] == "blocks" {
|
if depKind[i] == "blocks" {
|
||||||
@@ -350,14 +354,14 @@ func (s *Server) handleGetItemContext(w http.ResponseWriter, r *http.Request) {
|
|||||||
resp.Conflicts = append(resp.Conflicts, itemContextConflict{Kind: "stale_decision", Refs: []string{item.Ref},
|
resp.Conflicts = append(resp.Conflicts, itemContextConflict{Kind: "stale_decision", Refs: []string{item.Ref},
|
||||||
Detail: "decision " + d.QuestionKey + " was evaluated against an earlier state of " + item.Ref})
|
Detail: "decision " + d.QuestionKey + " was evaluated against an earlier state of " + item.Ref})
|
||||||
}
|
}
|
||||||
if !b.take("decision", "", d.ItemSeq, d) {
|
if !b.take("decision", item.Ref, d.QuestionKey, d.ItemSeq, d) {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
resp.Decisions = append(resp.Decisions, d)
|
resp.Decisions = append(resp.Decisions, d)
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, l := range others {
|
for _, l := range others {
|
||||||
if !b.take("link", l.Ref, 0, l) {
|
if !b.take("link", l.Ref, l.Title, 0, l) {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
resp.Links = append(resp.Links, l)
|
resp.Links = append(resp.Links, l)
|
||||||
@@ -389,23 +393,23 @@ func (s *Server) contextRelatedVisible(r *http.Request, workspaceID string) func
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// contextBudget admits parts in order until the first one that does not fit;
|
// contextBudget admits parts in order while they fit. A part that does not
|
||||||
// from then on every part is omitted, so order is never inverted by a
|
// fit is skipped WHOLE (never truncated), listed in `omitted` by part, ref and
|
||||||
// smaller part slipping in after a larger one was refused.
|
// title, and filling continues: one 32 KB convention must not crowd out every
|
||||||
|
// smaller part after it (lead dogfood, PLAN-3598). Callers order parts by
|
||||||
|
// relevance, so what is skipped is the least relevant that does not fit.
|
||||||
type contextBudget struct {
|
type contextBudget struct {
|
||||||
limit, used int
|
limit, used int
|
||||||
full bool
|
|
||||||
resp *itemContextResponse
|
resp *itemContextResponse
|
||||||
}
|
}
|
||||||
|
|
||||||
func (b *contextBudget) take(part, ref string, seq int64, v any) bool {
|
func (b *contextBudget) take(part, ref, title string, seq int64, v any) bool {
|
||||||
n := partSize(v)
|
n := partSize(v)
|
||||||
if !b.full && b.used+n <= b.limit {
|
if b.used+n <= b.limit {
|
||||||
b.used += n
|
b.used += n
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
b.full = true
|
b.resp.Omitted = append(b.resp.Omitted, itemContextOmitted{Part: part, Ref: ref, Title: title, Seq: seq, Size: n, Reason: "budget"})
|
||||||
b.resp.Omitted = append(b.resp.Omitted, itemContextOmitted{Part: part, Ref: ref, Seq: seq, Size: n, Reason: "budget"})
|
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -417,15 +421,25 @@ func partSize(v any) int {
|
|||||||
return len(raw)
|
return len(raw)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// intentAlsoTriggers widens an intent past its own trigger (lead dogfood,
|
||||||
|
// PLAN-3598): implementing a task includes committing and opening the PR, so
|
||||||
|
// the rules for those steps (CONVE-735's mandatory review is on-pr-create)
|
||||||
|
// belong in an implement brief. Each is added only when the workspace
|
||||||
|
// declares it.
|
||||||
|
var intentAlsoTriggers = map[string][]string{
|
||||||
|
"implement": {"on-commit", "on-pr-create"},
|
||||||
|
}
|
||||||
|
|
||||||
// resolveContextIntent maps an intent onto a trigger the workspace declares on
|
// resolveContextIntent maps an intent onto a trigger the workspace declares on
|
||||||
// its conventions: "on-<intent>" if declared, else "<intent>" itself. When
|
// its conventions: "on-<intent>" if declared, else "<intent>" itself, plus the
|
||||||
// neither is declared it returns "" and the valid intents.
|
// declared triggers intentAlsoTriggers adds. When the intent's own trigger is
|
||||||
func (s *Server) resolveContextIntent(conv *collections.TraitedCollection, intent string) (string, []string, error) {
|
// not declared it returns "" and the valid intents.
|
||||||
|
func (s *Server) resolveContextIntent(conv *collections.TraitedCollection, intent string) (string, []string, []string, error) {
|
||||||
var options []string
|
var options []string
|
||||||
if conv != nil {
|
if conv != nil {
|
||||||
c, err := s.store.GetCollection(conv.ID)
|
c, err := s.store.GetCollection(conv.ID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", nil, err
|
return "", nil, nil, err
|
||||||
}
|
}
|
||||||
if c != nil {
|
if c != nil {
|
||||||
var schema models.CollectionSchema
|
var schema models.CollectionSchema
|
||||||
@@ -447,13 +461,19 @@ func (s *Server) resolveContextIntent(conv *collections.TraitedCollection, inten
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
sort.Strings(valid)
|
sort.Strings(valid)
|
||||||
|
var also []string
|
||||||
|
for _, t := range intentAlsoTriggers[strings.TrimPrefix(intent, "on-")] {
|
||||||
|
if declared[t] {
|
||||||
|
also = append(also, t)
|
||||||
|
}
|
||||||
|
}
|
||||||
if declared["on-"+intent] {
|
if declared["on-"+intent] {
|
||||||
return "on-" + intent, valid, nil
|
return "on-" + intent, also, valid, nil
|
||||||
}
|
}
|
||||||
if declared[intent] {
|
if declared[intent] {
|
||||||
return intent, valid, nil
|
return intent, also, valid, nil
|
||||||
}
|
}
|
||||||
return "", valid, nil
|
return "", nil, valid, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *Server) contextAncestors(item *models.Item, visible func(string) bool) ([]models.Item, error) {
|
func (s *Server) contextAncestors(item *models.Item, visible func(string) bool) ([]models.Item, error) {
|
||||||
@@ -545,16 +565,63 @@ func (s *Server) contextConventions(r *http.Request, workspaceID string, conv *c
|
|||||||
Priority: contextFieldString(it.Fields, "priority"), Role: it.AgentRoleSlug, Content: it.Content,
|
Priority: contextFieldString(it.Fields, "priority"), Role: it.AgentRoleSlug, Content: it.Content,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
// Relevance order (lead dogfood, PLAN-3598), so the budget spends itself
|
||||||
|
// on what matters first: the intent's own triggers, then rules scoped to
|
||||||
|
// the caller's role, then the global always-on ones; within a tier,
|
||||||
|
// priority (must > should > nice), then the smaller body (more rules fit),
|
||||||
|
// then the ref's number. Never the ref string, which put CONVE-13 after
|
||||||
|
// CONVE-1287.
|
||||||
|
tier := func(c itemContextConvention) int {
|
||||||
|
switch {
|
||||||
|
case c.Trigger != "always":
|
||||||
|
return 0
|
||||||
|
case c.Role != "":
|
||||||
|
return 1
|
||||||
|
default:
|
||||||
|
return 2
|
||||||
|
}
|
||||||
|
}
|
||||||
sort.SliceStable(out, func(i, j int) bool {
|
sort.SliceStable(out, func(i, j int) bool {
|
||||||
pi, pj := conventionPriorityRank(out[i].Priority), conventionPriorityRank(out[j].Priority)
|
if ti, tj := tier(out[i]), tier(out[j]); ti != tj {
|
||||||
if pi != pj {
|
return ti < tj
|
||||||
|
}
|
||||||
|
if pi, pj := conventionPriorityRank(out[i].Priority), conventionPriorityRank(out[j].Priority); pi != pj {
|
||||||
return pi < pj
|
return pi < pj
|
||||||
}
|
}
|
||||||
return out[i].Ref < out[j].Ref
|
if li, lj := len(out[i].Content), len(out[j].Content); li != lj {
|
||||||
|
return li < lj
|
||||||
|
}
|
||||||
|
return refLess(out[i].Ref, out[j].Ref)
|
||||||
})
|
})
|
||||||
return out, nil
|
return out, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// refLess orders refs by prefix, then by their number as a number
|
||||||
|
// ("CONVE-13" before "CONVE-1287").
|
||||||
|
func refLess(a, b string) bool {
|
||||||
|
pa, na := splitRef(a)
|
||||||
|
pb, nb := splitRef(b)
|
||||||
|
if pa != pb {
|
||||||
|
return pa < pb
|
||||||
|
}
|
||||||
|
if na != nb {
|
||||||
|
return na < nb
|
||||||
|
}
|
||||||
|
return a < b
|
||||||
|
}
|
||||||
|
|
||||||
|
func splitRef(ref string) (string, int) {
|
||||||
|
i := strings.LastIndexByte(ref, '-')
|
||||||
|
if i < 0 {
|
||||||
|
return ref, 0
|
||||||
|
}
|
||||||
|
n, err := strconv.Atoi(ref[i+1:])
|
||||||
|
if err != nil {
|
||||||
|
return ref, 0
|
||||||
|
}
|
||||||
|
return ref[:i], n
|
||||||
|
}
|
||||||
|
|
||||||
func (s *Server) contextPlaybooks(r *http.Request, workspaceID string, traited []collections.TraitedCollection, triggers []string) ([]itemContextPlaybook, error) {
|
func (s *Server) contextPlaybooks(r *http.Request, workspaceID string, traited []collections.TraitedCollection, triggers []string) ([]itemContextPlaybook, error) {
|
||||||
pb := collections.FindByArtifactKind(traited, string(artifact.KindPlaybook))
|
pb := collections.FindByArtifactKind(traited, string(artifact.KindPlaybook))
|
||||||
if pb == nil {
|
if pb == nil {
|
||||||
@@ -576,7 +643,13 @@ func (s *Server) contextPlaybooks(r *http.Request, workspaceID string, traited [
|
|||||||
}
|
}
|
||||||
out = append(out, itemContextPlaybook{Ref: it.Ref, Title: it.Title, Trigger: trig, InvocationSlug: contextFieldString(it.Fields, "invocation_slug")})
|
out = append(out, itemContextPlaybook{Ref: it.Ref, Title: it.Title, Trigger: trig, InvocationSlug: contextFieldString(it.Fields, "invocation_slug")})
|
||||||
}
|
}
|
||||||
sort.SliceStable(out, func(i, j int) bool { return out[i].Ref < out[j].Ref })
|
// The intent's own triggers before always-on ones, then by ref number.
|
||||||
|
sort.SliceStable(out, func(i, j int) bool {
|
||||||
|
if ai, aj := out[i].Trigger == "always", out[j].Trigger == "always"; ai != aj {
|
||||||
|
return !ai
|
||||||
|
}
|
||||||
|
return refLess(out[i].Ref, out[j].Ref)
|
||||||
|
})
|
||||||
return out, nil
|
return out, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ package server
|
|||||||
import (
|
import (
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"sort"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
@@ -101,7 +102,9 @@ func TestPLAN3598_ItemContextBrief(t *testing.T) {
|
|||||||
if resp.Item.Ref != f.task.Ref || resp.Item.Seq == 0 || resp.Item.Content != "task body" {
|
if resp.Item.Ref != f.task.Ref || resp.Item.Seq == 0 || resp.Item.Content != "task body" {
|
||||||
t.Errorf("item = %+v", resp.Item)
|
t.Errorf("item = %+v", resp.Item)
|
||||||
}
|
}
|
||||||
if strings.Join(resp.Triggers, ",") != "always,on-implement" {
|
// implement also pulls the commit and PR rules the workspace declares
|
||||||
|
// (lead dogfood: CONVE-735, the mandatory review, is on-pr-create).
|
||||||
|
if tr := strings.Join(resp.Triggers, ","); !strings.HasPrefix(tr, "always,on-implement,on-commit") {
|
||||||
t.Errorf("triggers = %v", resp.Triggers)
|
t.Errorf("triggers = %v", resp.Triggers)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -114,15 +117,15 @@ func TestPLAN3598_ItemContextBrief(t *testing.T) {
|
|||||||
t.Errorf("acceptance criteria = %+v", ac)
|
t.Errorf("acceptance criteria = %+v", ac)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Conventions: always + on-implement, global + this role's; not
|
// Conventions: always + the implement triggers, global + this role's;
|
||||||
// on-commit, not another role's; must before should.
|
// not another role's; must before should.
|
||||||
refs := convRefs(resp.Conventions)
|
refs := convRefs(resp.Conventions)
|
||||||
for _, k := range []string{"always-must", "impl-should", "impl-must", "impl-implementer"} {
|
for _, k := range []string{"always-must", "impl-should", "impl-must", "impl-implementer", "commit"} {
|
||||||
if ctxIndex(refs, f.conv[k].Ref) < 0 {
|
if ctxIndex(refs, f.conv[k].Ref) < 0 {
|
||||||
t.Errorf("missing convention %s (%s) in %v", k, f.conv[k].Ref, refs)
|
t.Errorf("missing convention %s (%s) in %v", k, f.conv[k].Ref, refs)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
for _, k := range []string{"commit", "impl-reviewer"} {
|
for _, k := range []string{"impl-reviewer"} {
|
||||||
if ctxIndex(refs, f.conv[k].Ref) >= 0 {
|
if ctxIndex(refs, f.conv[k].Ref) >= 0 {
|
||||||
t.Errorf("convention %s should not be in the brief: %v", k, refs)
|
t.Errorf("convention %s should not be in the brief: %v", k, refs)
|
||||||
}
|
}
|
||||||
@@ -183,9 +186,11 @@ func TestPLAN3598_ItemContextRefusals(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
// A tiny budget keeps the item, omits the rest in order, and never lets a
|
// A tiny budget keeps the item and omits the rest, each named. A part that
|
||||||
// later part in once one was refused.
|
// does not fit is skipped WHOLE and filling continues, so a smaller later
|
||||||
func TestPLAN3598_ItemContextBudgetIsOrderedAndWhole(t *testing.T) {
|
// part still ships (lead dogfood: one 32 KB convention used to crowd out
|
||||||
|
// everything after it). The parent chain alone stops at its first miss.
|
||||||
|
func TestPLAN3598_ItemContextBudgetSkipsAndKeepsFilling(t *testing.T) {
|
||||||
forEachDialect(t, func(t *testing.T, d store.DriverType) {
|
forEachDialect(t, func(t *testing.T, d store.DriverType) {
|
||||||
f := newCtxFixture(t, d)
|
f := newCtxFixture(t, d)
|
||||||
full, _, _ := f.context(f.ownerTok, "?intent=implement")
|
full, _, _ := f.context(f.ownerTok, "?intent=implement")
|
||||||
@@ -204,21 +209,196 @@ func TestPLAN3598_ItemContextBudgetIsOrderedAndWhole(t *testing.T) {
|
|||||||
if len(resp.Omitted) == 0 || resp.Omitted[0].Part != "parent" || resp.Omitted[0].Ref != f.plan.Ref || resp.Omitted[0].Size == 0 {
|
if len(resp.Omitted) == 0 || resp.Omitted[0].Part != "parent" || resp.Omitted[0].Ref != f.plan.Ref || resp.Omitted[0].Size == 0 {
|
||||||
t.Errorf("omitted = %+v", resp.Omitted)
|
t.Errorf("omitted = %+v", resp.Omitted)
|
||||||
}
|
}
|
||||||
|
for _, o := range resp.Omitted {
|
||||||
|
if o.Ref == "" || o.Title == "" {
|
||||||
|
t.Errorf("an omitted entry does not name its ref and title: %+v", o)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// A budget that fits the item, the parent, and room for one small
|
// Room for the item, the parent and one small link, but not the
|
||||||
// later part (a link) but not the first convention: the convention is
|
// first convention. The convention is skipped, filling continues, and
|
||||||
// refused, and the link, which would fit, must not slip in after it.
|
// the result is FULL in the keep-filling sense: no omitted part would
|
||||||
|
// fit in what is left. (Stopping at the first miss breaks that: the
|
||||||
|
// link would fit the leftover and be omitted anyway.)
|
||||||
linkSize, convSize := partSize(full.Links[0]), partSize(full.Conventions[0])
|
linkSize, convSize := partSize(full.Links[0]), partSize(full.Conventions[0])
|
||||||
if linkSize >= convSize {
|
if linkSize >= convSize {
|
||||||
t.Fatalf("precondition: a link (%d) must be smaller than the first convention (%d)", linkSize, convSize)
|
t.Fatalf("precondition: a link (%d) must be smaller than the first convention (%d)", linkSize, convSize)
|
||||||
}
|
}
|
||||||
mid := itemOnly + partSize(full.Parents[0]) + linkSize
|
mid := itemOnly + partSize(full.Parents[0]) + linkSize
|
||||||
resp, _, _ = f.context(f.ownerTok, "?intent=implement&budget="+itoa(mid))
|
resp, _, _ = f.context(f.ownerTok, "?intent=implement&budget="+itoa(mid))
|
||||||
if len(resp.Parents) != 1 || len(resp.Conventions) != 0 || len(resp.Playbooks) != 0 || len(resp.Links) != 0 || len(resp.Deps.BlockedBy) != 0 || len(resp.Decisions) != 0 {
|
if len(resp.Parents) != 1 {
|
||||||
t.Errorf("mid budget: parents %d conventions %d playbooks %d links %d deps %d decisions %d", len(resp.Parents), len(resp.Conventions), len(resp.Playbooks), len(resp.Links), len(resp.Deps.BlockedBy), len(resp.Decisions))
|
t.Errorf("mid budget: parents %d, want 1", len(resp.Parents))
|
||||||
}
|
}
|
||||||
if len(resp.Omitted) == 0 || resp.Omitted[0].Part != "convention" || resp.Omitted[0].Ref != full.Conventions[0].Ref {
|
if ctxIndex(omittedRefs(resp.Omitted), full.Conventions[0].Ref) < 0 {
|
||||||
t.Errorf("mid budget omitted = %+v", resp.Omitted)
|
t.Errorf("mid budget: the first convention was not omitted: %+v", resp.Omitted)
|
||||||
|
}
|
||||||
|
if resp.Used <= itemOnly+partSize(full.Parents[0]) {
|
||||||
|
t.Errorf("mid budget: nothing shipped after the skipped convention (used %d)", resp.Used)
|
||||||
|
}
|
||||||
|
left := mid - resp.Used
|
||||||
|
for _, o := range resp.Omitted {
|
||||||
|
if o.Size <= left {
|
||||||
|
t.Errorf("omitted %s %s (%d) would fit the %d left: filling stopped early", o.Part, o.Ref, o.Size, left)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
func omittedRefs(os []itemContextOmitted) []string {
|
||||||
|
out := make([]string, len(os))
|
||||||
|
for i, o := range os {
|
||||||
|
out[i] = o.Ref
|
||||||
|
}
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
|
// Lead dogfood, PLAN-3598: a 32 KB convention ahead of small ones in the
|
||||||
|
// order is skipped, and the small ones after it still ship.
|
||||||
|
func TestPLAN3598_ItemContextBigConventionDoesNotBlockSmallOnes(t *testing.T) {
|
||||||
|
forEachDialect(t, func(t *testing.T, d store.DriverType) {
|
||||||
|
f := newCtxFixture(t, d)
|
||||||
|
conv, err := f.srv.store.GetCollectionBySlug(f.wsID, "conventions")
|
||||||
|
if err != nil || conv == nil {
|
||||||
|
t.Fatalf("conventions: %v", err)
|
||||||
|
}
|
||||||
|
// must on on-implement: the highest tier and priority, so it sorts
|
||||||
|
// ahead of every should rule whatever its size.
|
||||||
|
big, err := f.srv.store.CreateItem(f.wsID, conv.ID, models.ItemCreate{
|
||||||
|
Title: "Ctx huge rule", Content: strings.Repeat("x", 32*1024),
|
||||||
|
Fields: `{"status":"active","trigger":"on-implement","priority":"must","scope":"all"}`,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
resp, code, body := f.context(f.ownerTok, "?intent=implement&budget=20000")
|
||||||
|
if resp == nil {
|
||||||
|
t.Fatalf("status %d: %s", code, body)
|
||||||
|
}
|
||||||
|
refs := convRefs(resp.Conventions)
|
||||||
|
if ctxIndex(refs, big.Ref) >= 0 {
|
||||||
|
t.Fatalf("a 32 KB rule shipped under a 20000 budget")
|
||||||
|
}
|
||||||
|
if ctxIndex(omittedRefs(resp.Omitted), big.Ref) < 0 {
|
||||||
|
t.Errorf("the big rule is not listed as omitted: %+v", resp.Omitted)
|
||||||
|
}
|
||||||
|
for _, k := range []string{"impl-should", "always-must", "commit"} {
|
||||||
|
if ctxIndex(refs, f.conv[k].Ref) < 0 {
|
||||||
|
t.Errorf("small rule %s was crowded out by the big one: %v", k, refs)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// Lead dogfood, PLAN-3598: relevance order. The intent's triggers come
|
||||||
|
// first, then role-scoped rules, then the global always ones; within a
|
||||||
|
// tier, must before should. Never the ref string.
|
||||||
|
func TestPLAN3598_ItemContextConventionsInRelevanceOrder(t *testing.T) {
|
||||||
|
forEachDialect(t, func(t *testing.T, d store.DriverType) {
|
||||||
|
f := newCtxFixture(t, d)
|
||||||
|
resp, code, body := f.context(f.ownerTok, "?intent=implement&role=implementer")
|
||||||
|
if resp == nil {
|
||||||
|
t.Fatalf("status %d: %s", code, body)
|
||||||
|
}
|
||||||
|
tier := func(c itemContextConvention) int {
|
||||||
|
switch {
|
||||||
|
case c.Trigger != "always":
|
||||||
|
return 0
|
||||||
|
case c.Role != "":
|
||||||
|
return 1
|
||||||
|
}
|
||||||
|
return 2
|
||||||
|
}
|
||||||
|
// Equal-key conventions (same trigger, priority and body length)
|
||||||
|
// whose ref numbers straddle a digit boundary, so string order and
|
||||||
|
// numeric order disagree and only the ref NUMBER tiebreak passes.
|
||||||
|
conv, err := f.srv.store.GetCollectionBySlug(f.wsID, "conventions")
|
||||||
|
if err != nil || conv == nil {
|
||||||
|
t.Fatalf("conventions: %v", err)
|
||||||
|
}
|
||||||
|
var tied []string
|
||||||
|
for i := 0; i < 120; i++ {
|
||||||
|
it, err := f.srv.store.CreateItem(f.wsID, conv.ID, models.ItemCreate{
|
||||||
|
Title: "Ctx tie " + itoa(i), Content: "same body",
|
||||||
|
Fields: `{"status":"active","trigger":"on-implement","priority":"nice","scope":"all"}`,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
tied = append(tied, it.Ref)
|
||||||
|
if _, first := splitRef(tied[0]); i > 0 && len(itoa(first)) != len(it.Ref)-strings.LastIndexByte(it.Ref, '-')-1 {
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if _, a := splitRef(tied[0]); len(itoa(a)) == len(tied[len(tied)-1])-strings.LastIndexByte(tied[len(tied)-1], '-')-1 {
|
||||||
|
t.Fatalf("precondition: the tied refs %s..%s do not straddle a digit boundary", tied[0], tied[len(tied)-1])
|
||||||
|
}
|
||||||
|
resp, _, _ = f.context(f.ownerTok, "?intent=implement&role=implementer&budget=400000")
|
||||||
|
if got, want := tiedOrder(convRefs(resp.Conventions), tied), sortedByNumber(tied); strings.Join(got, ",") != strings.Join(want, ",") {
|
||||||
|
t.Errorf("equal-key conventions not in ref-number order:\n got %v\nwant %v", got, want)
|
||||||
|
}
|
||||||
|
|
||||||
|
cs := resp.Conventions
|
||||||
|
if len(cs) < 3 {
|
||||||
|
t.Fatalf("too few conventions to check the order: %v", convRefs(cs))
|
||||||
|
}
|
||||||
|
if cs[0].Trigger == "always" || conventionPriorityRank(cs[0].Priority) != conventionPriorityRank("must") {
|
||||||
|
t.Errorf("the first convention is not an intent-trigger must rule: %+v", cs[0])
|
||||||
|
}
|
||||||
|
for i := 1; i < len(cs); i++ {
|
||||||
|
a, b := cs[i-1], cs[i]
|
||||||
|
if tier(a) > tier(b) || (tier(a) == tier(b) && conventionPriorityRank(a.Priority) > conventionPriorityRank(b.Priority)) {
|
||||||
|
t.Errorf("%s (%s/%s) sorts before %s (%s/%s)", a.Ref, a.Trigger, a.Priority, b.Ref, b.Trigger, b.Priority)
|
||||||
|
}
|
||||||
|
// Within one tier and priority, the smaller body first.
|
||||||
|
if tier(a) == tier(b) && conventionPriorityRank(a.Priority) == conventionPriorityRank(b.Priority) && len(a.Content) > len(b.Content) {
|
||||||
|
t.Errorf("%s (%d bytes) sorts before the smaller %s (%d bytes)", a.Ref, len(a.Content), b.Ref, len(b.Content))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// A role-scoped rule on the intent's trigger is in the first tier:
|
||||||
|
// ahead of every always-on rule.
|
||||||
|
refs := convRefs(cs)
|
||||||
|
roleRule := ctxIndex(refs, f.conv["impl-implementer"].Ref)
|
||||||
|
if roleRule < 0 {
|
||||||
|
t.Fatalf("precondition: the role-scoped on-implement rule is missing from %v", refs)
|
||||||
|
}
|
||||||
|
for _, c := range cs {
|
||||||
|
if c.Trigger == "always" && roleRule > ctxIndex(refs, c.Ref) {
|
||||||
|
t.Errorf("the role-scoped on-implement rule sorts after always-on %s", c.Ref)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !refLess("CONVE-13", "CONVE-1287") || refLess("CONVE-1287", "CONVE-13") {
|
||||||
|
t.Error("refLess compares ref numbers as strings")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// Lead dogfood, PLAN-3598: implement includes opening the PR, so an
|
||||||
|
// on-pr-create rule (CONVE-735, the mandatory review) is in the brief.
|
||||||
|
func TestPLAN3598_ItemContextImplementIncludesPRRules(t *testing.T) {
|
||||||
|
forEachDialect(t, func(t *testing.T, d store.DriverType) {
|
||||||
|
f := newCtxFixture(t, d)
|
||||||
|
conv, err := f.srv.store.GetCollectionBySlug(f.wsID, "conventions")
|
||||||
|
if err != nil || conv == nil {
|
||||||
|
t.Fatalf("conventions: %v", err)
|
||||||
|
}
|
||||||
|
pr, err := f.srv.store.CreateItem(f.wsID, conv.ID, models.ItemCreate{
|
||||||
|
Title: "Ctx review before merge", Content: "run the review",
|
||||||
|
Fields: `{"status":"active","trigger":"on-pr-create","priority":"must","scope":"all"}`,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
resp, code, body := f.context(f.ownerTok, "?intent=implement")
|
||||||
|
if resp == nil {
|
||||||
|
t.Fatalf("status %d: %s", code, body)
|
||||||
|
}
|
||||||
|
if ctxIndex(convRefs(resp.Conventions), pr.Ref) < 0 {
|
||||||
|
t.Errorf("an on-pr-create must rule is missing from an implement brief: triggers %v, conventions %v", resp.Triggers, convRefs(resp.Conventions))
|
||||||
|
}
|
||||||
|
// Control: a commit brief does not pull it.
|
||||||
|
resp, _, _ = f.context(f.ownerTok, "?intent=commit")
|
||||||
|
if resp != nil && ctxIndex(convRefs(resp.Conventions), pr.Ref) >= 0 {
|
||||||
|
t.Errorf("a commit brief pulled the on-pr-create rule")
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -396,3 +576,24 @@ func TestPLAN3598_ItemContextBlockerAgreesWithDashboard(t *testing.T) {
|
|||||||
want("an open blocker whose collection schema does not decode", blockedBy("Ctx odd-blocked task", *odd), *odd, true)
|
want("an open blocker whose collection schema does not decode", blockedBy("Ctx odd-blocked task", *odd), *odd, true)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// tiedOrder is the order in which the refs in want appear in got.
|
||||||
|
func tiedOrder(got, want []string) []string {
|
||||||
|
in := map[string]bool{}
|
||||||
|
for _, r := range want {
|
||||||
|
in[r] = true
|
||||||
|
}
|
||||||
|
var out []string
|
||||||
|
for _, r := range got {
|
||||||
|
if in[r] {
|
||||||
|
out = append(out, r)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
|
func sortedByNumber(refs []string) []string {
|
||||||
|
out := append([]string(nil), refs...)
|
||||||
|
sort.Slice(out, func(i, j int) bool { return refLess(out[i], out[j]) })
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user