From 06f759c37b124462287d2970e6b2c091cfd3cc51 Mon Sep 17 00:00:00 2001 From: Sam Rolfe Date: Thu, 15 Oct 2026 19:54:26 +1100 Subject: [PATCH] Phase 8 follow-up: resolve the child named in the question The Hub forwards `person` equal to the asker (`PersonOrDefault`), so the spoken question "what does Harry have on today at school?" arrives with person "sam" and returned "no data for that person" (found by the Hub agent on AUD-116). resolveChild now falls back to the question text when the requested person is not one of this project's children: whole-word, case-insensitive match against the child names this service already publishes in /capabilities. An explicit person still wins, two named children still return answer:null, and the permission check still runs on the resolved child. No model, no free text. Live end-to-end through the Hub, with no person field: POST http://192.168.20.13:8788/ask {"text":"what does Harry have on today at school?","asker":"sam","source":"console"} 200 {"status":"ok", "answer":"Harry has 8 school events today from 8:55am to 6:00pm and 6 overdue tasks.", "route":{"kind":"project","project":"compass_nhs"}} Image 0.6.1-phase8; docs/phase-8-hub-notes.md records the fallback and the end-to-end transcript. Co-Authored-By: Paperclip --- Makefile | 4 +- README.md | 4 +- docker-compose.yml | 4 +- docs/phase-8-hub-notes.md | 81 ++++++++++++++++++++++++++---------- internal/hub/hub.go | 52 ++++++++++++++++++++++- internal/hub/hub_test.go | 74 +++++++++++++++++++++++++++++++- internal/web/hub_api_test.go | 42 +++++++++++++++++++ 7 files changed, 230 insertions(+), 31 deletions(-) diff --git a/Makefile b/Makefile index 0f5da71..5154dd5 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,5 @@ -IMAGE ?= compass-teams-nhs/school:0.6.0-phase8 -BUILD_VERSION ?= 0.6.0-phase8 +IMAGE ?= compass-teams-nhs/school:0.6.1-phase8 +BUILD_VERSION ?= 0.6.1-phase8 # Parse options (see scripts/parse-compass.sh). CHILD ?= Harry diff --git a/README.md b/README.md index f87fe37..e30a9ef 100644 --- a/README.md +++ b/README.md @@ -168,8 +168,8 @@ docker compose up -d --build Generate secrets with the built image: ```bash -docker run --rm --entrypoint /app/school compass-teams-nhs/school:0.6.0-phase8 gen-secret -docker run --rm --entrypoint /app/school compass-teams-nhs/school:0.6.0-phase8 hash-password 'the-password' +docker run --rm --entrypoint /app/school compass-teams-nhs/school:0.6.1-phase8 gen-secret +docker run --rm --entrypoint /app/school compass-teams-nhs/school:0.6.1-phase8 hash-password 'the-password' ``` Note: Docker Compose interpolates `$VAR` inside `env_file`. A bcrypt hash diff --git a/docker-compose.yml b/docker-compose.yml index 127a4bd..cbca7e8 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -10,8 +10,8 @@ services: build: context: . args: - VERSION: "0.6.0-phase8" - image: compass-teams-nhs/school:0.6.0-phase8 + VERSION: "0.6.1-phase8" + image: compass-teams-nhs/school:0.6.1-phase8 container_name: compass-teams-nhs restart: unless-stopped env_file: diff --git a/docs/phase-8-hub-notes.md b/docs/phase-8-hub-notes.md index a50c292..e91258d 100644 --- a/docs/phase-8-hub-notes.md +++ b/docs/phase-8-hub-notes.md @@ -89,6 +89,29 @@ Jo and Sam) may ask about any child, and anyone else may ask about themselves only. An unknown person returns `answer: null` with a reason; no data is leaked between the children. +### Which child the question is about + +The contract makes `person` optional and says it defaults to `asker`. The Hub +(as of AUD-116) forwards `person = asker`, so the spoken question *"what does +Harry have on today at school?"* arrives with `person: "sam"`. Because this +project is the expert on its own data, `resolveChild` falls back to the question +text when the requested person is not one of its children: + +- The requested `person` wins when it names a child (so an explicit `person` is + never overridden). +- Otherwise the question is split into whole words and matched, + case-insensitively, against the child names this service already publishes in + `/capabilities`. +- Exactly one match resolves the child; two matches ("what do Harry and Finn + have on?") return `answer: null` + a reason, because the contract answers for + one person at most. +- The permission check still runs on the resolved child. + +No model and no free-text guessing are involved, and a name is never matched +inside a longer word. This makes the Hub's current call shape answer correctly; +[AUD-117](/AUD/issues/AUD-117) (the Hub deriving `person` itself) remains a +useful Hub-side improvement, not a dependency of this phase. + ## `/notify` โ€” the interpretation The contract defines `POST /notify` as project-to-Hub (the *project* posts to the @@ -118,7 +141,31 @@ make verify-phase8 deploy/verify-phase8.sh --send ``` -## Evidence (2026-10-15, 08:46โ€“08:47 UTC / 19:46โ€“19:47 AEDT) +## Evidence (2026-10-15) + +### End-to-end through the live Hub (08:56 UTC / 19:56 AEDT) + +The literal phase question, sent to the **Hub** on its inbound API with no +`person` field, after [AUD-116](/AUD/issues/AUD-116) registered `compass_nhs`: + +``` +POST http://192.168.20.13:8788/ask +{"text":"what does Harry have on today at school?","asker":"sam","source":"console"} + +200 +{ + "status": "ok", + "request_id": "01M4ZC9G4TRXJ3MFSMJ66JSVQ1", + "answer": "Harry has 8 school events today from 8:55am to 6:00pm and 6 overdue tasks.", + "shape": "text", + "route": {"kind": "project", "project": "compass_nhs"}, + "expires": "2026-10-15T13:00:00Z" +} +``` + +This is the Phase 8 definition of done: the Hub asks, and gets a correct answer. + +### Against the service directly `deploy/verify-phase8.sh` on `.13` against `http://127.0.0.1:8792`: @@ -164,30 +211,20 @@ hub notify delivered from=verify-phase8 level=info recipient=Sam channel=email hub notify delivered from=verify-phase8 level=info recipient=Sam channel=whatsapp status=200 ``` -## The Hub wiring (remaining) +## The Hub wiring (done) -The Hub container (`central-ai-hub:local`, port 8788) currently runs with -`HUB_PROJECTS` empty, so it has no data project to route to. Registering this -service and restarting the Hub is a change to a running, family-used service and -a change to `~/.config/environment.d/10-secrets.conf`, so it is done only with a -recorded approval. +The Hub container (`central-ai-hub:local`, port 8788) now has this service +registered. The change was made by the Hub agent on +[AUD-116](/AUD/issues/AUD-116): -The change is exactly: +1. `~/.config/environment.d/10-secrets.conf` gained + `HUB_PROJECT_COMPASS_NHS_TOKEN` and `compass_nhs=http://192.168.20.13:8792` + in `HUB_PROJECTS` (alongside `email_agent` and `internet_control`). +2. The `hub` container was restarted once, bounded. +3. The transcript above is the result. -1. Add to `~/.config/environment.d/10-secrets.conf`: - `HUB_PROJECT_COMPASS_NHS_TOKEN=`. -2. Set `HUB_PROJECTS` there to include - `compass_nhs=http://192.168.20.13:8792` (keep the existing `email_agent` entry). -3. Restart the `hub` container: `docker compose -f /home/sam/paseo/projects/central_ai_llm_master/docker-compose.yml up -d hub`. - -Then the end-to-end check is: - -```bash -curl -sS -H "Authorization: Bearer $HUB_TOKEN_CONSOLE" -H 'Content-Type: application/json' \ - -d '{"text":"what does Harry have on today at school?","asker":"sam","source":"console"}' \ - http://192.168.20.13:8788/ask -# expect route.kind = project, route.project = compass_nhs, status ok -``` +`routing.md` was not changed: the `compass_nhs` row already existed and matches +this service's `/capabilities` name. ## Decisions and notes diff --git a/internal/hub/hub.go b/internal/hub/hub.go index f7a75f8..c3576d4 100644 --- a/internal/hub/hub.go +++ b/internal/hub/hub.go @@ -20,6 +20,7 @@ import ( "fmt" "strings" "time" + "unicode" "gitea.lab.audasmedia.com.au/sam/compass_teams_nhs/internal/dashboard" "gitea.lab.audasmedia.com.au/sam/compass_teams_nhs/internal/store" @@ -156,7 +157,7 @@ func (a *Answerer) Answer(ctx context.Context, req AskRequest) AskResponse { if err != nil { return cannotAnswer("the school data could not be read") } - child := findChild(children, person) + child := resolveChild(children, person, req.Question) if child == nil { return cannotAnswer("no data for that person") } @@ -415,6 +416,55 @@ func findChild(children []store.Child, name string) *store.Child { return nil } +// resolveChild picks the child a question is about. +// +// The contract makes `person` optional and says it defaults to `asker`. The Hub +// (as of AUD-116) forwards `person = asker`, so a spoken question such as "what +// does Harry have on today at school?" arrives as person "sam". This project is +// the expert on its own data: when the requested person is not one of its +// children, it looks for exactly one child named in the question text. The name +// must be one the service already publishes in /capabilities, the match is +// whole-word and case-insensitive, and the permission check still runs +// afterwards, so nothing is guessed and no data leaks between children. +func resolveChild(children []store.Child, person, question string) *store.Child { + if c := findChild(children, person); c != nil { + return c + } + words := wordsIn(question) + var named []*store.Child + for i := range children { + if words[normalize(children[i].Name)] { + named = append(named, &children[i]) + } + } + if len(named) == 1 { + return named[0] + } + return nil +} + +// wordsIn lower-cases a question and splits it into whole words, so "harry" +// matches in "what does Harry have on?" but never inside a longer word. +func wordsIn(s string) map[string]bool { + out := map[string]bool{} + var b strings.Builder + flush := func() { + if b.Len() > 0 { + out[b.String()] = true + b.Reset() + } + } + for _, r := range s { + if unicode.IsLetter(r) || unicode.IsDigit(r) { + b.WriteRune(unicode.ToLower(r)) + continue + } + flush() + } + flush() + return out +} + func onlyType(items []store.Item, typ string) []store.Item { out := make([]store.Item, 0, len(items)) for _, it := range items { diff --git a/internal/hub/hub_test.go b/internal/hub/hub_test.go index bad5d00..5d390a7 100644 --- a/internal/hub/hub_test.go +++ b/internal/hub/hub_test.go @@ -75,8 +75,12 @@ func harryData() *fakeReader { {ID: 9, ChildID: 1, Type: "task", Subject: "11MGMA MATHS", Title: "SAC 4: Networks", Body: "Status: Pending", DueAt: at(2026, 10, 19, 16, 0)}, {ID: 10, ChildID: 1, Type: "message", Title: "Health Information", DueAt: at(2026, 10, 13, 9, 0)}, - }}, - scans: map[int64]store.ScanStatus{1: okScan(testNow.Add(-4 * time.Hour))}, + }, + 2: { + {ID: 20, ChildID: 2, Type: "event", Title: "9:00: 10ENGA - B2 - SMI", DueAt: at(2026, 10, 15, 9, 0)}, + }, + }, + scans: map[int64]store.ScanStatus{1: okScan(testNow.Add(-4 * time.Hour)), 2: okScan(testNow.Add(-4 * time.Hour))}, } } @@ -233,6 +237,72 @@ func TestEmptyQuestionCannotAnswer(t *testing.T) { } } +// TestChildNamedInQuestionResolvesThePerson covers the Hub's actual call shape: +// the Hub sends `person` equal to the asker, so the child's name is only in the +// question. The literal phase question must still answer. +func TestChildNamedInQuestionResolvesThePerson(t *testing.T) { + a := testAnswerer(harryData()) + resp := a.Answer(context.Background(), AskRequest{ + Question: "what does Harry have on today at school?", + Asker: "sam", + Person: "sam", // what the Hub forwards today (person defaults to asker) + Source: "console", + }) + if resp.Answer == nil { + t.Fatalf("answer = nil, reason = %q", resp.Reason) + } + if resp.Data["child"] != "Harry" { + t.Errorf("data.child = %v, want Harry", resp.Data["child"]) + } + if !strings.Contains(*resp.Answer, "school event") { + t.Errorf("answer = %q, want the lesson count", *resp.Answer) + } +} + +// TestExplicitChildWinsOverQuestionName: a caller that names a child wins, even +// if the question text mentions another one. +func TestExplicitChildWinsOverQuestionName(t *testing.T) { + a := testAnswerer(harryData()) + resp := a.Answer(context.Background(), AskRequest{ + Question: "what does Harry have on today?", + Asker: "sam", + Person: "finn", + }) + if resp.Answer == nil { + t.Fatalf("answer = nil, reason = %q", resp.Reason) + } + if resp.Data["child"] != "Finn" { + t.Errorf("data.child = %v, want Finn", resp.Data["child"]) + } +} + +// TestTwoChildrenNamedCannotAnswer: the contract answers for one person at +// most, so an ambiguous question is refused rather than guessed. +func TestTwoChildrenNamedCannotAnswer(t *testing.T) { + a := testAnswerer(harryData()) + resp := a.Answer(context.Background(), AskRequest{ + Question: "what do Harry and Finn have on today?", + Asker: "sam", + Person: "sam", + }) + if resp.Answer != nil { + t.Fatalf("answer = %q, want nil for an ambiguous question", *resp.Answer) + } +} + +// TestNameIsNotMatchedInsideAWord keeps the fallback to whole words. +func TestNameIsNotMatchedInsideAWord(t *testing.T) { + a := testAnswerer(harryData()) + resp := a.Answer(context.Background(), AskRequest{ + Question: "what does harrysaurus have on today?", + Asker: "sam", + Person: "sam", + }) + if resp.Answer != nil { + t.Fatalf("answer = %q, want nil (no whole-word child name)", *resp.Answer) + } +} + func TestCapabilitiesNameMatchesRouting(t *testing.T) { caps := DefaultCapabilities() if caps.Project != "compass_nhs" { diff --git a/internal/web/hub_api_test.go b/internal/web/hub_api_test.go index bdb49ff..be39a7a 100644 --- a/internal/web/hub_api_test.go +++ b/internal/web/hub_api_test.go @@ -142,6 +142,48 @@ func TestHubCapabilitiesAndAsk(t *testing.T) { } } +// TestHubAskResolvesTheNamedChild covers the Hub's actual call shape: the Hub +// sends `person` equal to the asker (`PersonOrDefault`), so the child's name is +// only in the question text. The literal phase question must still answer. +func TestHubAskResolvesTheNamedChild(t *testing.T) { + ts, st := newTestServerHub(t, testAppriseConfig("", "", ""), testHubToken) + loc := mustLoc(t) + ctx := context.Background() + harry, _ := st.ChildByName(ctx, "Harry") + now := time.Now().In(loc) + today := time.Date(now.Year(), now.Month(), now.Day(), 9, 0, 0, 0, loc) + addHubItem(t, st, harry.ID, store.Item{ + Type: "event", Subject: "11XMUSA6", Teacher: "JBR", + Title: "1 - 11XMUSA6 - A1 - JBR", DueAt: &today, + }) + if _, err := st.RecordScan(ctx, &harry.ID, 5, 1, true, ""); err != nil { + t.Fatalf("record scan: %v", err) + } + + resp, raw := hubJSON(t, ts, http.MethodPost, "/ask", testHubToken, + `{"question":"what does Harry have on today at school?","asker":"sam","person":"sam","source":"console"}`) + if resp.StatusCode != http.StatusOK { + t.Fatalf("ask status = %d, body = %s", resp.StatusCode, raw) + } + var ask struct { + Answer *string `json:"answer"` + Reason string `json:"reason"` + Data map[string]any `json:"data"` + } + if err := json.Unmarshal(raw, &ask); err != nil { + t.Fatalf("ask json: %v (%s)", err, raw) + } + if ask.Answer == nil { + t.Fatalf("answer = nil, reason = %q", ask.Reason) + } + if ask.Data["child"] != "Harry" { + t.Errorf("data.child = %v, want Harry", ask.Data["child"]) + } + if !strings.Contains(*ask.Answer, "school event") { + t.Errorf("answer = %q, want the lesson count", *ask.Answer) + } +} + // TestHubAskReadsOnly is the contract's first rule made testable: /ask and // /capabilities must not write anything (AGENTS.md ยง10 rule 5). func TestHubAskReadsOnly(t *testing.T) {