From 1d079a8bf12c0a5a1b2bc5f0a29b94d9683c2c80 Mon Sep 17 00:00:00 2001 From: Sam Rolfe Date: Thu, 15 Oct 2026 19:54:26 +1100 Subject: [PATCH] AUD-117: derive person from the question via project people list Decode the people list from each project's /capabilities and set the /ask person field from a whole-word, case-insensitive name in the question when the caller did not name a person. Match only the routed project's people, so a name from another project does not leak across. Fall back to asker when no name matches. Co-Authored-By: Paperclip --- docs/outline/03-code-map.md | 2 +- routing.md | 7 ++- src/destinations/project.go | 26 +++++--- src/hub/hub.go | 79 +++++++++++++++++++----- src/hub/hub_test.go | 92 +++++++++++++++++++++++++++- src/route/route.go | 2 +- src/tests/fakeproject/fakeproject.go | 17 +++++ src/understand/person.go | 37 +++++++++++ src/understand/person_test.go | 39 ++++++++++++ 9 files changed, 273 insertions(+), 28 deletions(-) create mode 100644 src/understand/person.go create mode 100644 src/understand/person_test.go diff --git a/docs/outline/03-code-map.md b/docs/outline/03-code-map.md index 7d2a55c..8d2becc 100644 --- a/docs/outline/03-code-map.md +++ b/docs/outline/03-code-map.md @@ -32,7 +32,7 @@ central_ai_llm_master/ ids/ ULID request and approval ids config/ environment and secrets loading hear/ step 1: decode and validate the request - understand/ step 2: never-route check and the routing LLM + understand/ step 2: never-route check, person match, routing LLM check/ step 3: identity and source checks route/ step 4: dispatch to one destination answer/ step 5: reply shaping (text, speech, notify) diff --git a/routing.md b/routing.md index 9095b56..ca72a57 100644 --- a/routing.md +++ b/routing.md @@ -17,8 +17,8 @@ For each request, the Hub runs the checks in this order: 1. **Never-route check.** If the request is a control action (section 3), return `status: "refused"`. Stop. Do not ask an LLM. -2. **Read capabilities.** Read the topic list from each project's - `/capabilities` (`docs/ask-contract.md`). +2. **Read capabilities.** Read the topic list and the people list from each + project's `/capabilities` (`docs/ask-contract.md`). 3. **Read this file.** Read the topic map (section 1) and the overlap rule (section 2). 4. **Ask the LLM.** Give the LLM the question, the capability lists, and this @@ -28,6 +28,9 @@ For each request, the Hub runs the checks in this order: - `clarify` — ask the user one short question. - `no_match` — say it does not know. 5. **Route.** Send the request. The action call holds no LLM (brief rule 4). + When the caller did not name a `person`, set it from the question: match a + name against the routed project's `people` list, case-insensitive and whole + word. When no name matches, `person` stays the `asker`. A project name in this file must match the `project` value in that project's `/capabilities` reply. A new project adds one row here and one `/capabilities` diff --git a/src/destinations/project.go b/src/destinations/project.go index 7408ae5..782168e 100644 --- a/src/destinations/project.go +++ b/src/destinations/project.go @@ -90,33 +90,43 @@ func (p *ProjectClient) Ask(ctx context.Context, body AskRequest) (AskReply, err return reply, nil } -// Capabilities reads the project's topic list. It is best effort: a project -// that is down does not stop the Hub. -func (p *ProjectClient) Capabilities(ctx context.Context) ([]string, error) { +// Capabilities is a project's published lists from GET /capabilities +// (docs/ask-contract.md). Topics route the question. People let the Hub set +// the /ask person field from the question. +type Capabilities struct { + Project string + Topics []string + People []string +} + +// Capabilities reads the project's topic and people lists. It is best effort: +// a project that is down does not stop the Hub. +func (p *ProjectClient) Capabilities(ctx context.Context) (Capabilities, error) { ctx, cancel := context.WithTimeout(ctx, p.Timeout) defer cancel() req, err := http.NewRequestWithContext(ctx, http.MethodGet, p.URL+"/capabilities", nil) if err != nil { - return nil, err + return Capabilities{}, err } if p.Token != "" { req.Header.Set("Authorization", "Bearer "+p.Token) } resp, err := p.HTTP.Do(req) if err != nil { - return nil, err + return Capabilities{}, err } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("project %s: capabilities status %d", p.Name, resp.StatusCode) + return Capabilities{}, fmt.Errorf("project %s: capabilities status %d", p.Name, resp.StatusCode) } var out struct { Project string `json:"project"` Topics []string `json:"topics"` + People []string `json:"people"` } if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { - return nil, err + return Capabilities{}, err } - return out.Topics, nil + return Capabilities{Project: out.Project, Topics: out.Topics, People: out.People}, nil } diff --git a/src/hub/hub.go b/src/hub/hub.go index 758eaab..ec23a6a 100644 --- a/src/hub/hub.go +++ b/src/hub/hub.go @@ -15,6 +15,7 @@ import ( "hub/answer" "hub/apierr" "hub/config" + "hub/destinations" "hub/hear" "hub/logbook" "hub/route" @@ -36,9 +37,17 @@ type Service struct { capMu sync.Mutex capTopics []string + capPeople map[string][]string capFetched time.Time } +// capabilitySnapshot is the live project ability list, keyed by project for +// people so a name from one project never leaks into another. +type capabilitySnapshot struct { + topics []string + people map[string][]string +} + // CapabilityTTL is how long the Hub keeps a project capability list. const CapabilityTTL = 30 * time.Second @@ -51,10 +60,11 @@ func (s *Service) Ask(ctx context.Context, req hear.Request) answer.Response { if understand.IsControlAction(req.Text) { outcome = route.Refused() } else { + caps := s.capabilities(ctx) decision, err := s.Router.Decide(ctx, understand.Input{ Question: req.Text, Person: req.PersonOrDefault(), - Topics: s.capabilityTopics(ctx), + Topics: caps.topics, Rules: s.Rules, }) if err != nil { @@ -65,6 +75,7 @@ func (s *Service) Ask(ctx context.Context, req hear.Request) answer.Response { Reason: err.Error(), } } else { + req = applyPerson(req, decision, caps.people) outcome = s.Dispatcher.Dispatch(ctx, decision, req) } } @@ -83,48 +94,51 @@ func (s *Service) Ask(ctx context.Context, req hear.Request) answer.Response { return resp } -// capabilityTopics reads the live capability topics from every project, with a -// short cache. A project that is down contributes nothing and does not block. -func (s *Service) capabilityTopics(ctx context.Context) []string { +// capabilities reads the live capability topics and people from every project, +// with a short cache. A project that is down contributes nothing and does not +// block. +func (s *Service) capabilities(ctx context.Context) capabilitySnapshot { if s.Dispatcher == nil || len(s.Dispatcher.Projects) == 0 { - return nil + return capabilitySnapshot{} } s.capMu.Lock() if time.Since(s.capFetched) < CapabilityTTL && s.capTopics != nil { - topics := s.capTopics + snap := capabilitySnapshot{topics: s.capTopics, people: s.capPeople} s.capMu.Unlock() - return topics + return snap } s.capMu.Unlock() type result struct { - topics []string + project string + caps destinations.Capabilities } results := make([]result, 0, len(s.Dispatcher.Projects)) var mu sync.Mutex var wg sync.WaitGroup - for _, client := range s.Dispatcher.Projects { + for name, client := range s.Dispatcher.Projects { wg.Add(1) - go func(c route.ProjectQuerier) { + go func(name string, c route.ProjectQuerier) { defer wg.Done() cctx, cancel := context.WithTimeout(ctx, 1*time.Second) defer cancel() - topics, err := c.Capabilities(cctx) + caps, err := c.Capabilities(cctx) if err != nil { return } mu.Lock() - results = append(results, result{topics: topics}) + results = append(results, result{project: name, caps: caps}) mu.Unlock() - }(client) + }(name, client) } wg.Wait() var topics []string seen := map[string]bool{} + people := map[string][]string{} for _, r := range results { - for _, t := range r.topics { + for _, t := range r.caps.Topics { t = strings.ToLower(strings.TrimSpace(t)) if t == "" || seen[t] { continue @@ -132,13 +146,48 @@ func (s *Service) capabilityTopics(ctx context.Context) []string { seen[t] = true topics = append(topics, t) } + for _, person := range r.caps.People { + person = strings.ToLower(strings.TrimSpace(person)) + if person == "" { + continue + } + people[r.project] = append(people[r.project], person) + } } s.capMu.Lock() s.capTopics = topics + s.capPeople = people s.capFetched = time.Now() s.capMu.Unlock() - return topics + return capabilitySnapshot{topics: topics, people: people} +} + +// applyPerson sets req.Person from a household name in the question when the +// caller did not name a person. It matches only the routed project's published +// people list, so a name from another project does not leak across. When no +// name matches, PersonOrDefault still falls back to the asker (docs/hub-api.md +// section 5). +func applyPerson(req hear.Request, decision understand.Decision, people map[string][]string) hear.Request { + if req.Person != "" { + return req + } + var projects []string + switch decision.Action { + case understand.ActionRoute: + projects = []string{decision.Project} + case understand.ActionFanout: + projects = decision.Projects + default: + return req + } + for _, p := range projects { + if name := understand.MatchPerson(req.Text, people[p]); name != "" { + req.Person = name + return req + } + } + return req } // LogAsk writes one log line for a completed request (docs/hub-api.md section diff --git a/src/hub/hub_test.go b/src/hub/hub_test.go index 0e0c292..bd69dfe 100644 --- a/src/hub/hub_test.go +++ b/src/hub/hub_test.go @@ -82,7 +82,7 @@ func (p *fakePaperclip) FileJob(ctx context.Context, title, description string) func newTestService(t *testing.T, decider understand.Decider) (*Service, *fakeproject.Project, *recordingSpeaker, *fakePaperclip) { t.Helper() - fp := &fakeproject.Project{Name: "compass_nhs", Topics: []string{"school", "homework", "timetable"}} + fp := &fakeproject.Project{Name: "compass_nhs", Topics: []string{"school", "homework", "timetable"}, People: []string{"harry", "finn"}} srv := fp.Server() t.Cleanup(srv.Close) @@ -135,6 +135,96 @@ func TestAskRoutesThroughLLMToFakeProject(t *testing.T) { } } +// newMultiProjectService wires two fake projects so a test can prove a name +// published by one project does not leak into the other. +func newMultiProjectService(t *testing.T, decider understand.Decider) (*Service, *fakeproject.Project, *fakeproject.Project) { + t.Helper() + compass := &fakeproject.Project{Name: "compass_nhs", Topics: []string{"school"}, People: []string{"harry"}, Answers: map[string]string{"school": "School ends at 3pm today."}} + compassSrv := compass.Server() + t.Cleanup(compassSrv.Close) + email := &fakeproject.Project{Name: "email_agent", Topics: []string{"email"}, People: []string{"finn"}, Answers: map[string]string{"finn": "Finn has two emails."}} + emailSrv := email.Server() + t.Cleanup(emailSrv.Close) + + svc := &Service{ + Config: &config.Config{}, + Rules: understand.DefaultRules(), + Router: decider, + Dispatcher: &route.Dispatcher{ + Rules: understand.DefaultRules(), + Projects: map[string]route.ProjectQuerier{ + "compass_nhs": destinations.NewProjectClient("compass_nhs", compassSrv.URL, "", time.Second), + "email_agent": destinations.NewProjectClient("email_agent", emailSrv.URL, "", time.Second), + }, + Speaker: &recordingSpeaker{}, + }, + Started: time.Now(), + } + return svc, compass, email +} + +func TestAskDerivesPersonFromQuestion(t *testing.T) { + dec := &recordingDecider{decide: understand.Decision{Action: understand.ActionRoute, Project: "compass_nhs"}} + svc, fp, _, _ := newTestService(t, dec) + + req := hear.Request{Text: "what does Harry have on today at school?", Asker: "sam", Source: hear.SourceConsole, RequestID: "r7"} + resp := svc.Ask(context.Background(), req) + + if resp.Status != "ok" { + t.Fatalf("status = %q (%s)", resp.Status, resp.Answer) + } + if got := fp.Persons(); len(got) != 1 || got[0] != "harry" { + t.Fatalf("project persons = %v, want [harry]", got) + } +} + +func TestAskPersonDefaultsToAskerWhenNoName(t *testing.T) { + dec := &recordingDecider{decide: understand.Decision{Action: understand.ActionRoute, Project: "compass_nhs"}} + svc, fp, _, _ := newTestService(t, dec) + + req := hear.Request{Text: "what is on at school today?", Asker: "sam", Source: hear.SourceConsole, RequestID: "r8"} + resp := svc.Ask(context.Background(), req) + + if resp.Status != "ok" { + t.Fatalf("status = %q (%s)", resp.Status, resp.Answer) + } + if got := fp.Persons(); len(got) != 1 || got[0] != "sam" { + t.Fatalf("project persons = %v, want [sam]", got) + } +} + +func TestAskPersonNameFromAnotherProjectNotMatched(t *testing.T) { + dec := &recordingDecider{decide: understand.Decision{Action: understand.ActionRoute, Project: "compass_nhs"}} + svc, compass, _ := newMultiProjectService(t, dec) + + // finn is published by email_agent only. The question routes to compass_nhs, + // so compass_nhs must not receive finn. + req := hear.Request{Text: "what does finn have on today at school?", Asker: "sam", Source: hear.SourceConsole, RequestID: "r9"} + resp := svc.Ask(context.Background(), req) + + if resp.Status != "ok" { + t.Fatalf("status = %q (%s)", resp.Status, resp.Answer) + } + if got := compass.Persons(); len(got) != 1 || got[0] != "sam" { + t.Fatalf("compass persons = %v, want [sam]", got) + } +} + +func TestAskPersonMatchesOwnProject(t *testing.T) { + dec := &recordingDecider{decide: understand.Decision{Action: understand.ActionRoute, Project: "email_agent"}} + svc, _, email := newMultiProjectService(t, dec) + + req := hear.Request{Text: "what does finn have on today?", Asker: "sam", Source: hear.SourceConsole, RequestID: "r10"} + resp := svc.Ask(context.Background(), req) + + if resp.Status != "ok" { + t.Fatalf("status = %q (%s)", resp.Status, resp.Answer) + } + if got := email.Persons(); len(got) != 1 || got[0] != "finn" { + t.Fatalf("email persons = %v, want [finn]", got) + } +} + func TestControlActionNeverReachesLLM(t *testing.T) { dec := &recordingDecider{decide: understand.Decision{Action: understand.ActionNoMatch}} svc, _, _, _ := newTestService(t, dec) diff --git a/src/route/route.go b/src/route/route.go index 02347c0..327ef92 100644 --- a/src/route/route.go +++ b/src/route/route.go @@ -76,7 +76,7 @@ type Outcome struct { // ProjectQuerier is one data project. *destinations.ProjectClient implements it. type ProjectQuerier interface { Ask(ctx context.Context, body destinations.AskRequest) (destinations.AskReply, error) - Capabilities(ctx context.Context) ([]string, error) + Capabilities(ctx context.Context) (destinations.Capabilities, error) } // Announcer starts house TTS. *destinations.Speaker implements it. diff --git a/src/tests/fakeproject/fakeproject.go b/src/tests/fakeproject/fakeproject.go index 7a7d31c..d6deb5c 100644 --- a/src/tests/fakeproject/fakeproject.go +++ b/src/tests/fakeproject/fakeproject.go @@ -25,6 +25,9 @@ var DefaultAnswers = map[string]string{ type Project struct { Name string Topics []string + // People is the household list the fake project publishes, and the list it + // accepts in the /ask person field. + People []string // Answers maps a question keyword to a short answer. Nil uses // DefaultAnswers. @@ -37,6 +40,7 @@ type Project struct { mu sync.Mutex questions []string + persons []string } // Ask records that the Hub reached the project. @@ -55,6 +59,15 @@ func (p *Project) Questions() []string { return out } +// Persons returns the person field the Hub sent, in order. +func (p *Project) Persons() []string { + p.mu.Lock() + defer p.mu.Unlock() + out := make([]string, len(p.persons)) + copy(out, p.persons) + return out +} + // Handler returns the three /ask-contract endpoints. func (p *Project) Handler() http.Handler { mux := http.NewServeMux() @@ -69,6 +82,7 @@ func (p *Project) Handler() http.Handler { writeJSON(w, http.StatusOK, map[string]any{ "project": name, "topics": p.Topics, + "people": p.People, }) }) mux.HandleFunc("/ask", p.handleAsk) @@ -94,6 +108,9 @@ func (p *Project) handleAsk(w http.ResponseWriter, r *http.Request) { http.Error(w, "bad json", http.StatusBadRequest) return } + p.mu.Lock() + p.persons = append(p.persons, body.Person) + p.mu.Unlock() p.Ask(body.Question) if p.Delay > 0 { diff --git a/src/understand/person.go b/src/understand/person.go new file mode 100644 index 0000000..2829fbc --- /dev/null +++ b/src/understand/person.go @@ -0,0 +1,37 @@ +package understand + +import ( + "regexp" + "strings" +) + +// MatchPerson returns the household name from people that appears as a whole +// word in the question, or "" when no name matches. +// +// It is deterministic: no LLM is in this path (AGENTS.md rule 4). The Hub uses +// it to set the /ask person field from the question when the caller did not +// name a person. A caller-supplied person always wins; the caller does not call +// this function in that case. +// +// The match is case-insensitive and whole-word, so "harry" matches "What does +// Harry have on today?" but not "harrypotter". The matched name is returned as +// the project published it, trimmed. +func MatchPerson(question string, people []string) string { + if strings.TrimSpace(question) == "" || len(people) == 0 { + return "" + } + q := strings.ToLower(question) + for _, p := range people { + name := strings.TrimSpace(p) + if name == "" { + continue + } + // \b is a word boundary in Go's RE2. QuoteMeta keeps a name that + // holds a regexp character literal. + re := regexp.MustCompile(`\b` + regexp.QuoteMeta(strings.ToLower(name)) + `\b`) + if re.MatchString(q) { + return name + } + } + return "" +} diff --git a/src/understand/person_test.go b/src/understand/person_test.go new file mode 100644 index 0000000..492e717 --- /dev/null +++ b/src/understand/person_test.go @@ -0,0 +1,39 @@ +package understand + +import "testing" + +func TestMatchPerson(t *testing.T) { + people := []string{"harry", "finn"} + cases := []struct { + name string + question string + want string + }{ + {"capitalised", "What does Harry have on today at school?", "harry"}, + {"lowercase", "what does harry have on today?", "harry"}, + {"possessive", "what is Harry's timetable?", "harry"}, + {"second name", "has finn done his homework?", "finn"}, + {"no name", "what is the weather today?", ""}, + {"substring is not a word", "harrypotter is a film", ""}, + {"empty question", "", ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := MatchPerson(tc.question, people); got != tc.want { + t.Fatalf("MatchPerson(%q) = %q, want %q", tc.question, got, tc.want) + } + }) + } +} + +func TestMatchPersonNoPeople(t *testing.T) { + if got := MatchPerson("what does Harry have on today?", nil); got != "" { + t.Fatalf("MatchPerson with no people = %q, want empty", got) + } +} + +func TestMatchPersonRegexpNameIsLiteral(t *testing.T) { + if got := MatchPerson("does a.b have homework?", []string{"a.b"}); got != "a.b" { + t.Fatalf("MatchPerson literal dot = %q, want %q", got, "a.b") + } +}