PROMOTE develop → main (lp612 prod go-live) — DO NOT MERGE without operator go - #676
Draft
hyasin270 wants to merge 948 commits into
Draft
PROMOTE develop → main (lp612 prod go-live) — DO NOT MERGE without operator go#676hyasin270 wants to merge 948 commits into
hyasin270 wants to merge 948 commits into
Conversation
lp612: per-family author model + flash-tier brief — the dsflash maths/physics pilot (bd-u6za9)
… handler
CI caught what my machine could not. `test (22)` failed on
`Cannot find module 'aws-sdk'` while every local run was green, because my
worktree borrows another worktree's root node_modules — which contains aws-sdk
even though the root package.json does not declare it. CI's `npm ci` installs
only what is declared, so the dependency simply is not there.
The chain is short: text-message.handler requires lesson-plan-queue, which
requires ./queue, which loads the SQS driver, which requires 'aws-sdk' at module
scope. Any root suite that loads the real text handler therefore dies — and that
is precisely why the handler's existing sibling suite asserts against SOURCE TEXT
instead of loading the module. A source-text assertion cannot catch a wiring bug
that only exists at runtime, which is the whole point of the new suite, so the
dependency is stubbed rather than the test weakened.
Follows the documented convention for this repo ("add a bot dependency, add its
stub in the same PR") and the shape of the @aws-sdk/client-s3, axios, pino and
exceljs stubs beside it. Surface is exactly what the bot touches —
AWS.config.update() and new AWS.SQS() — and SQS records its calls rather than
no-opping, so a future test can assert what would have been enqueued.
This also clears the cascade: tests/textbook-lp-v2/handler-curriculum.test.js
was reported as newly failing, but it passes on a pristine build of the same base
commit and fails only when a sibling suite dies in the same jest worker.
Verified: full lp612 552/552; the two suites the gate flagged show IDENTICAL
failure counts on a pristine base build (6 and 5), so neither is mine; and every
remaining gate delta is a line-number shift of an entry the snapshot already
records — text-message.handler.js:138/162/173/187/293 shifted by one require
line. Zero new failing suites, zero genuine new offenders.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ulls in
Second CI failure, same root cause as the first: `Cannot find module 'uuid'`.
Loading the real text-message handler reaches 254 files, and seven bare packages
in that graph live only in bot/node_modules — which CI has not installed when the
root suite runs.
Rather than iterate against CI one missing package at a time, the require graph
was walked statically to get the complete set in one pass:
uuid, p-limit, sharp, bullmq, chartjs-node-canvas and the speech SDK
(async_hooks is a Node builtin, and aws-sdk is already stubbed globally).
They are mocked with `{ virtual: true }` — which mocks a module that does not
exist on disk at all, something a plain jest.mock cannot do — and kept LOCAL to
this suite. Only a test that loads the whole handler needs them, so the shared
moduleNameMapper should not grow six entries to serve one file.
AND THE REASON THIS TOOK TWO ROUNDS: this worktree borrowed another worktree's
root node_modules, which carries aws-sdk and uuid even though the root
package.json declares neither. Every local run was therefore green while CI
failed. Replaced with a real root `npm ci`, so the worktree now installs exactly
what CI installs.
Verified under a faithful CI simulation — real root npm ci, bot/node_modules
absent: handler-wiring 4/4 green, and the full baseline gate reports NO new
failing suites. The gate still exits 1 on line-number-shifted offenders, each
matched to an entry the snapshot already records.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Seven defects in the serving/authoring lane, each ending the same way for the
teacher: told her lesson is being written, and nothing ever arrives.
1. The worker delivered from a PRE-AUTHOR SNAPSHOT of `waiters`, then wrote
`waiters: []` over the real list. V1.3.2 made the append atomic; the drop
simply moved downstream. Every teacher who joined during the 2-10 minutes of
authoring was appended correctly, never read, then erased. New RPC
`lp612_claim_waiters` (V1.3.4) returns the list and empties it under one row
lock, called AFTER the terminal status is written so a concurrent join is
either included or refused with 'not_authoring' and re-decided into a cache
hit. Applied to the success path, the failure path and the follow-up message.
2. The insert-race loser discarded joinWaiters' return value: if the winner had
moved to ready/failed in between, no waiter was appended and she was still
told "already being written" and booked as joined. Both paths now share
joinInFlight(), which obeys the answer and retries a transient 'error' once.
3. findRender() could not tell "no row" from "read failed", so a transient error
fell through to an INSERT for a row that may exist, and turned a 23505 — which
is PROOF the winner exists — into "it failed". Now a discriminated result.
4. The retry/reset updated on `.eq('id')` alone: two taps both won, both enqueued
(~$1.50 and minutes each), and the second overwrote the first tapper out of
the waiter list. Now a compare-and-swap guarded on the status and started_at
it read; the loser joins the winning run, and existing waiters are preserved
rather than overwritten.
5. The FIFO MessageDeduplicationId omitted lang and renderId, so the en and ur
jobs for one segment differed only in the millisecond — a collision silently
drops a different lesson. lp612 now passes an explicit deduplicationId via the
existing opts parameter; the shared default is untouched.
6. lp612_join_waiters existed only in the V1.3.2 migration, so a fresh
bootstrap:db clone got the tables without the function and dropped every
waiter. Added to 00_complete-schema.sql with lp612_claim_waiters. The guard
that should have caught this was blind: its reference pattern was `[a-z_]+`,
which matches no digit, so no lp612_* RPC was ever scanned. Fixed, with a
test for the blind spot itself.
7. STALE_RECOVERY_INTERVAL_MINUTES documented beside the LP612 vars — it bounds
how long a stranded render lies about its state (~30 min on prod defaults).
Default unchanged.
Red-first throughout: 21 new tests across tests/lp612/golden-path-races.test.js
and tests/lp612/author-worker.test.js, each run red before its fix. The
author-worker assertion that pinned `waiters: []` encoded defect 1 and was
updated deliberately.
lp612 suites: 34 passed / 515 tests (was 33 / 494).
Closes: bd-pfest
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
feat(lp612): recognise a teacher's reply to a 6-12 lesson (phase 1, edit lane flag-off)
…ck if it fails Phase 2 core. Still unwired and inert: nothing calls this yet, and the lane's flag stays off. It lives INSIDE lp612-author.service.js on purpose — buildUserPrompt, buildRevisionPrompt, callWithRetry and runGates are already in scope there, so the whole feature adds no duplication. Her instruction enters through the same `notes` channel the revision preamble already ranks above every gate finding; that channel existed for operator-named defects and had never been fed at runtime. THE REJECTION CONTRACT is the part that matters, and the prototype got it wrong in a way that would have shipped broken lessons. To let a second round repair what the first broke, the loop keeps climbing from the candidate — so on a final rejected round the working document is the broken one. The prototype returned exactly that with `accepted: false` beside it, one careless `if (out.lpDoc)` away from sending a teacher the document the gates had just refused. So the original is deep-copied up front and returned unchanged on EVERY failure path — rejection, transport death, unparseable reply, schema break. Four tests assert it byte-for-byte rather than "looks similar". Acceptance is ABSOLUTE, not relative: the authoring ladder's notWorse() compares two candidates chasing one target, but here the incumbent is a document she already has and which already renders. The bar is "introduces no NEW blocking defect" — measured against HER document, so a lesson that already runs long is still editable, and an edit is never entitled to cost her a working lesson just by improving on its own last attempt. Round 1 carries her instruction; repair rounds carry a different note that says the edit is already applied and must be preserved. Re-asking for "shorter homework" against an already-shortened document is how a lesson gets cut twice, and repair rounds are the common path — 7 of the 12 measured cells needed one. 19 tests, red-first. Full lp612 lane 41 suites / 598 tests green, verified with bot/node_modules absent so the run matches CI rather than my machine. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… pristine
The shared render is keyed (segment_id, lang, template_version) with NO user
dimension: every teacher who taps that subtopic is served the same bytes. So an
edit written back onto it would rewrite the national lesson to suit one
teacher's homework preference. An edit forks; it never overwrites.
lp612/{tv}/{lang}/edits/{segment_id}/{hash}.pdf (+ .lp.json beside it)
Still under lp612/, so the shared-bucket prefix guard covers the new keys
unchanged — this bucket is shared with PK production and that guard is the only
isolation there is. template_version still leads, so bumping it expires a
version's forks alongside the parents they came from instead of orphaning them.
THE HASH IS CONTENT-ADDRESSED, AND user_id IS DELIBERATELY NOT IN IT. Two
teachers who ask the same thing of the same document get one render instead of
two, and a retry after a dropped connection lands on work already done. Which
teachers hold which edit is a database question for the edits table, not the
storage path's job. (The design note said "content-addressed, not
user-addressed" while listing user_id among the hash inputs — contradictory;
this resolves it, and the parameter stays in the signature so a caller passing
it is not silently wrong.)
The SOURCE DOCUMENT is hashed too, so an edit of an already-edited lesson cannot
collide with an edit of the original. The instruction is normalised so trivial
variants share a render, and hashed so her words never reach an object key —
prose in a key is both a path-safety and a privacy problem.
One real bug caught by its own test: dots are kept in the sanitiser because a
real segment id is `grade_8_mathematics.c05.p071-073`, which left `..` intact —
`../../pre_gen_lps/x` sanitised to `.._.._pre_gen_lps_x` and was then rejected at
the put. Runs of dots are now collapsed, which removes the traversal and leaves
every legitimate id untouched.
12 tests. Full lp612 lane 42 suites / 610 tests green, bot/node_modules absent
so the run matches CI.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
feat(lp612): reviseLessonPlan + fork keys (phase 2 core, still inert)
THE PILOT WAS INERT IN PRODUCTION AND 27 GREEN TESTS SAID OTHERWISE.
After the previous commit deployed to staging, the first live authoring of a
Grade 9 physics segment came back `model_used = "anthropic/claude-sonnet-5"`.
The maths/physics lane was never entered.
Root cause, one line: `lp612-author.worker.js` resolved the model ITSELF as
`resolveAuthorModel()` with no family, and passes it EXPLICITLY into
authorLessonPlan({ segment, lang, model, … }). The service's
`model || resolveAuthorModel(family)` therefore always took the worker's
family-less value, so the family branch was unreachable from the only call path
that runs in production.
This is the SAME class of defect as the one the previous commit fixed inside the
service — a second private model resolution, one layer up — and my own test
suite could not see it, because it called authorLessonPlan the way the worker
does NOT: without a model. A red test has to execute the PRODUCTION call path,
and "the caller passes this argument" is part of that path.
Fix: the worker resolves familyForBook(segment.book_stem) — the segment is
already loaded two lines above — and asks for that family's model.
tests/lp612/family-worker-wiring.test.js drives Worker.process() and asserts on
both the model handed to the author service AND the model written to the render
row (what an audit actually reads back). Red output was exactly the staging
symptom: Expected "deepseek/deepseek-v4-flash", Received "anthropic/claude-sonnet-5".
Three of its five tests passed before the fix — the scoping invariants — which is
what makes the two that failed evidence.
Also closes a gap this bead opened: the three vendored family flash briefs are
now in vendor-integrity's REQUIRED manifest, with a check that each is strictly
larger than brief_author_v3.md. They are GENERATED upstream, so a re-vendor that
regenerates v3 and forgets them would leave the flash lane authoring against a
brief that no longer matches the canon — a silent harness fork.
36 lp612 suites green, 532 tests.
The NIETE gitleaks job has failed on EVERY PR since #574 on a single finding, and it is a false positive: bot/scripts/import-lp612-segments.js:92 rule generic-api-key · entropy 3.511 · secret "niete_lp612_renders" match: key, `niete_lp612_renders` The line is a prose comment explaining a schema decision — "segment_id is the primary key, `niete_lp612_renders` carries a foreign key to it". The default generic-api-key rule fires on a KEYWORD followed by a quoted high-entropy-looking value, and ordinary English about a primary key does exactly that. The captured "secret" is a table name that is already in the public schema file. WHY THIS IS A SECURITY FIX AND NOT A COSMETIC ONE. A scanner that is always red is not a scanner. Seven-plus PRs have now been merged through a failing secret check, which trains everyone to treat the next finding — a real one — as noise too. The permanently-red state is the actual vulnerability. KEPT NARROW, three ways, rather than disabling the rule or excluding the file: * targetRules = ["generic-api-key"] — every explicit key-shape rule (Anthropic, OpenRouter, Supabase, Google, OpenAI, and both credential-assignment rules) is untouched; * regexTarget = "secret" — matched against the captured value, not the line, so a real key on the same line is still reported; * regex ^niete_[a-z0-9_]+$ — only our own snake_case Postgres identifiers. Real keys from every vendor we use carry mixed case, digits and dashes and cannot match this shape. Also converts the existing singular [allowlist] to the plural [[allowlists]] form, unchanged in content: gitleaks refuses a config that mixes the two ("[allowlist] is deprecated, it cannot be used alongside [[allowlists]]"). VERIFIED, because "the scanner is quiet now" is exactly the wrong success criterion for a change to a secret scanner: * canary file with six synthetic vendor keys — all six still caught (anthropic, openrouter, supabase, google, credential-assignment-uuid, credential-assignment-longtoken); * the SAME comment shape with a non-niete value ("key, `zqf7Kd91PlmXvT3RwNb8Ehy2`") STILL fires generic-api-key, proving the rule is live and only our identifiers are exempt; * full repo scan: no leaks found, exit 0.
Hazard raised by the bake-off lane: their runner auto-detected ANTHROPIC_API_KEY
from the environment and silently rerouted to api.anthropic.com, so results
produced on Anthropic were SCORED AS OPENROUTER until an explicit backend pin was
added. The damage was not the routing — it was a batch of measurements labelled
with the wrong provider, which is the kind of error you discover long after you
have acted on it.
This lane is already immune by construction: routing is keyed on an explicit
`anthropic-direct/` model prefix, never on key presence, and getClient() is
untouched. But "immune by construction" is a property that a later refactor can
delete silently, so it is now asserted rather than argued.
Three tests, no production change:
* setting ANTHROPIC_API_KEY changes NOTHING about default routing — provider,
baseURL, default model and getClient()'s baseURL are compared byte-for-byte
with and without the key. The only permitted difference is the
`anthropicDirectConfigured` advertisement, which reports CONFIGURED, never
IN USE;
* four model-id shapes — including `anthropic/claude-sonnet-5` and a bare
`claude-sonnet-5` — all still resolve to OpenRouter with the key present;
* the prefix is the only thing that switches lanes, and it is asserted against
the exported constant rather than a stringly-typed literal.
11/11 green in this suite.
security: unstick the permanently-red gitleaks check (bd-iab48) + prove the grant lane is explicit opt-in (bd-yoc6i)
lp612: the WORKER must resolve the author model per family — the pilot was inert (bd-u6za9)
Resolves three things.
1. A RAW NUL BYTE IN develop's lp612-serving.service.js MADE THE FILE UNMERGEABLE.
git reported "Cannot merge binary files" and produced NO conflict markers --
because develop's copy contains a literal 0x00 at line 113, inside a template
literal used as a hash separator:
.update(`${norm}<0x00>${source}`)
The intent is right (a NUL separator stops field-boundary collisions in a
hash) but it was written as the raw byte instead of the escape. It is
valid JS, so it runs and every test passes -- and git's binary heuristic then
treats the whole file as binary, which means NOBODY can merge or rebase across
it and no conflict can ever be displayed. That is a standing hazard for a busy
merge train, not a cosmetic issue.
Replaced with the escape: byte-identical at runtime, and the file is
text again. With the NUL gone, a real 3-way merge of the same two versions
reports ZERO conflicts -- the two lanes never actually touched the same lines.
The "conflict" was entirely an artefact of the binary detection.
2. .env.template -- kept both lanes' additions (LP612_EDIT_INTENT_MODEL from the
edit-flow lane, STALE_RECOVERY_INTERVAL_MINUTES from this one).
Changed this branch's recommendation from "leave UNSET in production" to
STALE_RECOVERY_INTERVAL_MINUTES=2. The operator's instruction was to align the
reaper to something teacher-humane, and the default is not: on 15 minutes the
worst case is ~30 minutes during which a stranded row tells every later tapper
the lesson is already being written and nothing ever arrives. The sweep is one
indexed query on a small table, so running it often costs nothing.
3. tests/lp612/family-worker-wiring.test.js (bd-u6za9) needed `rpc` on its
supabase double, because the worker now claims its delivery audience through
lp612_claim_waiters. Harness update, no behaviour change -- the suite still
asserts exactly what it did.
44 lp612 suites green, 642 tests.
lp612: harden the runtime-render + cache golden path against same-second races (bd-pfest)
The first native-Urdu render printed, on page 1 directly under the masthead:
previous: grade_10_urdu.p1c01.r990
`sequence.previous` is rendered verbatim by the template (lib/template.js:1043),
so an internal corpus id was painted onto a lesson plan a teacher prints and
carries into a classroom.
IT IS A PROMPT DEFECT, NOT A MODEL FAILURE. buildUserPrompt handed the model
where this sits: previous <prev_segment_id> - next <next_segment_id>
raw internal ids, labelled with the EXACT words of the teacher-facing output
fields. The model did the reasonable thing and copied them through. Any model
would; the prompt asked for it.
Fixed in both halves, because only one of them is load-bearing:
* the prompt no longer labels internal ids with output-field names. They are
still supplied as ordering context, but marked INTERNAL and paired with an
explicit instruction to write the topic name, or null, in `sequence`;
* sanitizeSequence() runs in CODE before the gates, on the first parse and on
every revision round, exactly where sanitizeOverlay runs. A prompt
instruction is not an input contract -- the model complies almost always and
freestyles the rest -- and that is the failure mode root CLAUDE.md rule 24(c)
exists for.
It never invents. The three NULLABLE fields (previous, next, checkpoint) are
dropped. `this` is required with minLength 3, so nulling it would make the
document schema-invalid and cost the entire ladder round -- a worse outcome than
the leak -- and it is instead replaced with the segment's own title, which we
already hold on the row. With no title available the id stays and the schema gate
reports it, because inventing a lesson name would be worse than either.
Detection is exact first, heuristic second: an exact match against the three ids
we actually passed, then a deliberately tight shape check for an id the model
invented (lowercase, dotted, no spaces). A test asserts that legitimate titles --
"Section 1.2 - Physical quantities", "Ch. 3 assessment (day 12)", and Urdu --
survive untouched, because a sanitiser that eats real content is worse than the
bug it fixes.
9 red-first tests; red was `sanitizeSequence is not a function`.
45 lp612 suites green, 651 tests.
lp612: stop internal segment ids reaching the teacher-facing sequence strip (bd-w56zx)
…y (bd-86ivw)
Operator: "Make sure telemetry is enabled in the lessons incl on staging so we
can get teacher feedback."
WHICH LOGGER SHIPS. `logToFile` already reaches the external backend: it routes
through console.log/warn/error (logger.js:59-66), which structured-logger
overrides into pino (structured-logger.js:447-470) writing to a dual-output
stream that feeds the Axiom batcher (structured-logger.js:232-253). Both entry
points require structured-logger first (whatsapp-bot.js:2, sqs-worker.js:32).
So none of it was churned. What the lane lacked was a stable EVENT NAME — a
sentence cannot be counted, grouped or alerted on, and it changes the day
somebody improves the wording.
PART A — one semantic event per stage, on the repo's existing
`feature.action.result` taxonomy (logEvent), additive beside every prose line:
lp612.tap.received pakistan-lp-endpoint serveLp612()
lp612.serve.<outcome> serving requestLesson(), one per REQUEST — the
recursion runs on the inner fn so a re-decide is
not double-counted
lp612.author.completed author service, with model/family/tier/rounds
lp612.render.completed render service, phase=gate|final so a ladder probe
lp612.render.failed is distinguishable from the teacher's document
lp612.deliver.completed worker, terminal, with the full provenance triple
lp612.deliver.failed worker, same triple + errorCode + elapsedMs
A FAILED RENDER NOW RECORDS ITS MODEL. Two rows sat at AUTHOR_TIMEOUT with
model_used NULL on 2026-09-03: `model_used` was written only inside the success
patch, so the one question a failed row exists to answer — pilot model, or
sonnet? — could not be answered. fail() takes the model the worker already
resolved from the segment's family, and family/tier ride the event (the renders
table has no columns for them, and adding two was not worth it).
NO ENV GATING on the logging path. The only branch is
`NODE_ENV !== 'production' && !RAILWAY_ENVIRONMENT` — on Railway staging
RAILWAY_ENVIRONMENT is set, so the Axiom stream is selected exactly as on prod.
Staging emits iff its bot + sqs-worker services carry AXIOM_DATASET and
AXIOM_TOKEN and LOG_LEVEL is not raised above info.
PART B — the survey, on lp_feedback, NOT a new table (rule 15). That table
already holds this exact shape for K-5 and its lesson_plan_id is nullable,
which is what makes room for a lane whose lessons are renders rather than
lesson_plans rows. It gains ONE nullable column (lp612_segment_id) and reuses
lp_variant as the lane+language discriminator ('lp612_en' / 'lp612_ur').
Ruled out, in the migration header: a new lp612_feedback table (eleven
duplicated columns and two queries to keep in step by hand), reusing
lesson_plan_id (UUID FK, cannot hold a text segment id), overloading topic, and
a second column for the document language.
Scheduled from inside deliverRender — the ONE function the worker's waiter loop
and the cache hit both run through, and the cache hit is the path most teachers
are on. Asked once per (teacher, lesson), checked in the delayed callback so a
teacher waiting for her PDF pays nothing for it. Every string from the catalog
via resolveUx in en+ur, capped in CODE POINTS (button 20) by an extension to
the existing tests/lp612 cap suite. Button ids lp612_fb_(yes|no)_(en|ur)_<seg>,
dispatched in whatsapp-bot.js beside their siblings and the reason consumer
wired into text-message.handler ahead of the 6-12 edit router, so an answer to
"which part did not work?" is never read as an instruction to rewrite her
lesson.
Schema lands in BOTH V1.3.5 and 00_complete-schema.sql — a migration-only
column is invisible to `npm run bootstrap:db`, which is the defect bd-pfest
just fixed. The vendored data-standards validator passes on the staged
migration.
TESTS: 4 files, red-first, each driving the production caller rather than a
helper — the endpoint (not serveLp612, which is not exported), Worker.process,
requestLesson, authorLessonPlan, renderLessonPlan. tests/lp612 is 49 suites /
707 tests green.
GATE: `npm test` exits 1, exactly as it does on the base commit 37b1ccb
(verified in a pristine worktree). Zero suites fail here that do not fail
there; three known-flaky training suites fail there and not here. The 933
source-hygiene offender lines are byte-identical in content between the two
runs — the only delta is line numbers shifting in the four already-offending
files this change edits.
Closes: bd-86ivw
lp612: queryable telemetry for every stage + a teacher-feedback survey (bd-86ivw)
serveLp612Segment -- the `lp612_serve` step, reached by tapping «اردو» or
«English» on the language screen -- returned a hardcoded English sentence:
'Your lesson plan is on its way -- check this chat in a moment.'
So a teacher who had just tapped «اردو» was answered in English, on the last
screen she sees before the Flow closes.
Its sibling a few lines above in the same file already did the right thing, so
this is now literally the same call:
resolveUx('lp612FlowAck', { language: who.preferred_language })
TWO DEFECTS IN ONE STRING.
LANGUAGE -- teacher-addressed text that never went through the catalog, which
root CLAUDE.md rule 20 forbids outright: one writer, every teacher-facing
string resolved at send time.
TIMING -- "in a moment" is the stale promise bd-2ym0h removed everywhere else,
and it is untrue here: a first hit on this lane measured 227-843 s on staging
yesterday. The catalog string says "as soon as it is ready", which stays
honest at any latency.
HER UI LANGUAGE, NOT `d.lang`, AND THAT IS THE POINT. `d.lang` is the DOCUMENT
she ordered; this screen is addressed TO her. The language-menu suite already
states the split in its own header -- "`lang` (the document) and `uiLang` (the
acks) -- the two territories diverge the moment an Urdu-UI teacher orders an
English physics plan" -- so an Urdu teacher ordering an English lesson is still
answered in Urdu. A test pins exactly that case.
6 red-first tests, including one asserting the two taps now return an identical
string, and a source guard so the literal cannot come back under a refactor.
Red output was the bug verbatim: expected the Urdu catalog string, received
"Your lesson plan is on its way -- check this chat in a moment."
50 lp612 suites green, 713 tests.
lp612: the language-row tap must answer in her language (bd-15l6c)
The operator, on the menu that shipped from bd-3uiev: "re: menu, it is inconsistent. Some chapters have their menu in the smaller subtitle field, some in the upper field. All of them need to have it in the consistent format so it fits chapter name and looks coherent too." bd-3uiev ported the K-5 rule: merge the name into the title when the whole thing fits in 30 code points, move it to the metadata line when it does not. That rule loses no information, passed every test, and still shipped a menu that reads as broken. LENGTH IS A PROPERTY OF THE DATA, so a length-conditional layout makes the shape of a row depend on which book the teacher opened. Grade 9 Physics rendered six names in the bold title and three in the small grey metadata, with the three short rows missing a third line entirely — one list that looks like two. It is worse across books than within one: grade 6 Mathematics (names averaging 16 code points) put almost every name in the title while grade 7 Agricultural Education (averaging 44) put every one in metadata. Same builder, same release, two menus that look like different products. The fix is removing the condition, not tuning the threshold. Every row now: number in the title, lesson count in the description, chapter NAME on the metadata line — including the names short enough to have fitted above. Which field holds the name is settled by the caps, not by taste: title is 30 code points, description is 20, and 277 of 761 chapter names in this corpus exceed 30. Metadata's 80 is the only field that reliably holds a real chapter name. Two consequences taken deliberately. The prominent line no longer carries a name — the price of one shape when a third of names cannot fit there. And books that repeat a chapter number (grade 11 Urdu's p1c01/p2c01/p3c01, grade 6 English's c01a/c01b) now show the same title twice; that is fine BECAUSE the name is always rendered, so every row is told apart on the line every row has rather than on whichever field it happened to use. bd-3uiev disambiguated those by putting the name back in the title, which is the very inconsistency being removed here. The subtopic list was audited for the same failure mode and does not have it: the lesson name is always the title there, never relocated by length. Proof, over the whole corpus through the real builder: 618 chapter rows across 53 books, 0 books whose rows differ in field SHAPE, 0 books where the NAME changes field, 0 truncated titles, 0 cap violations. The render proof grew those two checks and an offline --from-json mode so the comparison can be rebuilt from a snapshot. Three tests in catalog-long-names.test.js encoded the superseded rule. They are revised rather than deleted, and the one that asserted a short name merges into the title now asserts it deliberately does NOT — so nothing quietly reintroduces the special case. Closes: bd-tnvpg
fix(lp612): one row shape for every chapter, in every book (bd-tnvpg)
… extension (bd-awqt3) A one-shot 900s extendJobTimeout() at the top of the lp612_author job does not outlast the job in practice. LP612_AUTHOR_TIMEOUT_MS only bounds authoring + render inside lp612-author.worker.js's withTimeout() — the PDF read, both R2 uploads, the DB writes and the per-waiter WhatsApp delivery loop all run after that and are unbounded. Staging also runs that timeout at up to 840s on one worker vs 720s on another, leaving as little as 60s of margin. A load test measured jobs running past 936s, 36s past the 900s window: the message goes visible again, a second worker claims the same lesson, and duplicate authoring doubles the load exactly when contention was already the problem. Fix: bot/shared/utils/sqs-visibility-heartbeat.js is a small reusable heartbeat that re-extends visibility every ~60s for as long as a job is actually running, stops the instant it settles (success or failure, via try/finally so it can never leak), swallows a single failed extension without abandoning the job, and is bounded by an absolute ceiling (2x the job's own hard timeout) so a genuinely hung job still becomes visible again eventually. Wired to exactly one caller: the lp612_author case in workers/sqs-worker.js. No other job type's behaviour changes. Also (bd-awqt3 part 2): lp612_author has no dedicated SQS queue of its own — it rides `main` — so sqs-worker and sqs-worker-video both polling `main` (the WORKER_QUEUES default when unset) let the same lesson land on either service's differing LP612_AUTHOR_TIMEOUT_MS/LP612_AUTHOR_ROUNDS with nothing logged anywhere. resolveWorkerQueuesBootStatus() + a boot-time warning make that condition visible instead of silent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…erer as clean (bd-htueq)
Three changes for the P0 latency/quality-gate issue:
1. lp612-render.service.js gets a per-process FIFO semaphore
(LP612_RENDER_CONCURRENCY, default 2) around the vendored renderer's
Chromium browser launch. Up to 5 renders per lesson x SQS_WORKER_CONCURRENCY
(3) lessons per worker meant up to ~15 unbounded concurrent browser
instances fighting one container's CPU/RAM/64MB-/dev/shm - the measured
cause of the load-test latency blowup. The slot is released in a finally so
a throwing render never permanently burns it (covered explicitly by
render-concurrency.test.js).
2. renderLessonPlan's thrown errors now carry .infra (true = the renderer
died for its own reasons - a crash, a launch failure, a temp-file write
failure; false = a real document defect it validated and rejected: schema,
overlay, OVERFLOW/TYPE FLOOR/PAGE COUNT/TRUNCATION).
3. lp612-author.worker.js's renderCheck no longer collapses every thrown error
into [] ("no defects"), which silently told the revision ladder the
page-cap gate had passed when a crash meant it was never checked. An infra
failure now gets one retry (the semaphore makes this the common recoverable
case); if it still won't render, the gate returns an explicit, always
non-empty, always-blocking RENDER_INFRA: defect and emits
lp612.render.gate_infra_unresolved - the ladder can never again mistake
"unverified" for "clean". A real content defect is unaffected (single call,
returned verbatim, same as before).
Also adds lp612.render.slow telemetry (>2x LP612_RENDER_EXPECTED_MS),
reporting queueWaitMs and renderMs separately since a slow queue wait is a
capacity signal and a slow render is a contention signal.
Note: this is the LP612 lesson-plan renderer (bot/vendor/lp-v9), a separate
pipeline from the coaching-report PDF renderer maintained elsewhere in the
workspace - no font/bidi/language surface touched here.
Verified pre-existing on origin/develop, not caused by this change: npm test
already reports a REGRESSION verdict on a clean checkout (unrelated
observe-*/column-completeness/env-template offenders). Diffed the "NEW
offenders" section byte-for-byte between a clean checkout and this branch -
identical. This branch's own new env vars (LP612_RENDER_CONCURRENCY,
LP612_RENDER_EXPECTED_MS) are documented in .env.template and do not appear
as offenders.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n (bd-m1xyt) sendDocumentByLink never throws — every failure (Meta 5xx, rate limit, an expired 24h window, a bad token) is caught internally and returned as a plain `false`. deliverRender ignored that return: recordDelivery ran, the feedback prompt was scheduled, and the worker's per-waiter loop counted a `delivered` for a teacher who received nothing. Every surface reported success for a lesson that never left the building. Fix: the document send is retried with a small, bounded budget (3 attempts, seconds-scale backoff — production evidence from Axiom shows 97% of real failures are Meta's 131056 pair-rate-limit, which a fast retry only deepens) sharing ONE deadline across a whole delivery loop so a pile-up cannot extend a job past its SQS visibility window. Exhausting the budget makes deliverRender throw, which both existing callers already catch (cache-hit -> deliver_failed, worker loop -> deliveryFailures). recordDelivery() and the feedback prompt now run ONLY after a send that actually succeeded, and every failure emits a queryable lp612.send.failed event naming phone/renderId/segmentId/lang/attempts. render.status stays 'ready' on a failed send (kept conservative per the brief): the PDF is genuinely fine in R2 and a later tap serves it from cache in ~1s; flipping status would destroy a good cache entry to record a fact the new event already captures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-jddcu)
runGates() short-circuits on a schema failure, so a schema-invalid gate
result carries ONLY schema errors — lint and the render probe never ran,
so they never added their own defects to its count. notWorse() compared
candidates on raw defect count, which can only favour that undercounted
side: a schema-broken candidate with 1 defect could read as "cheaper"
than a schema-valid candidate carrying 3 lint/render defects, so a
document the renderer cannot even open could win the round and reach
the renderer. Live case: a coach-test lesson failed "SCHEMA INVALID —
refusing to render: /sections/1/blocks/2 must have required property
'text'; must NOT have additional properties ('ref')" after 332s.
Fix: schema validity is now a categorical tier checked ahead of the
existing lexicographic defect-count comparison in notWorse() — a
schema-valid candidate always beats a schema-invalid one, regardless of
defect counts on either side. Only within the same tier does the
existing "fewer BLOCKING defects wins, ties go to total count" rule
decide, unchanged. The schema short-circuit itself is preserved (lint
assumes schema-valid shape; the renderer throws on a broken one).
The edit lane (reviseLessonPlan) had the identical vulnerability in its
`blockingCost(g2) <= bar` acceptance check — found while fixing the
ladder, not currently wired to a live caller but fixed the same way for
the same reason.
Visibility: every schema failure now emits `lp612.author.schema_invalid`
(round, error count, sample errors), and a candidate discarded
specifically for being schema-invalid while the kept document was valid
additionally emits `lp612.author.schema_candidate_rejected` — so how
often the model emits an unrenderable document, and how often that
actually costs a round, are both measurable instead of requiring a human
to read one render-service log line by hand.
Tests: tests/lp612/author-schema-tier.test.js, driven through the real
authorLessonPlan()/reviseLessonPlan() with only the LLM mocked at the
network boundary — schema validation and lint run for real. Full
tests/lp612/ suite (52 suites/733 tests) green. Baseline-gate diffed
against a fresh origin/develop@af9bd33 checkout: identical "NEW
offenders" set (all four already-known-stale suites), zero new
offenders attributable to this branch, no new failing suite.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The paper arrives finished and unchangeable. If two questions are wrong for
her class, the only remedy is to build another paper from scratch and hope.
The schema has anticipated this since V1.2.6 — exam_json as the living
document, original_exam_json frozen beside it, selected_question_ids, and
edited_at — and nothing was ever built on those columns. This builds it.
After the document lands she is offered a Flow that lists her questions,
every one ticked. She unticks what she does not want, taps once, and the
same paper comes back without them. No model call: the questions already
exist, so this is a filter, a render and a send, and it answers in seconds
rather than the minute generation takes.
Three things carry the risk, and each has tests:
· A question is addressed by its PATH in the tree (seen.objective.MCQs.0),
never by its printed position. Untick question 2 and everything after it
renumbers, so a stored "question 4" would come to mean a different
question than the one she chose. A test pins this traversal against the
renderer's, because her ticks are numbered against what the paper printed.
· Her ticks live in the session, not in the form. Meta renders at most 20
checkbox options and real papers on staging came back at 10, 20, 28 and
64 questions, so the screen pages — and the form only ever knows the
twenty rows on screen. Merging per page is what stops a submit from an
early page silently dropping every question she never scrolled to.
· NULL and [] stay distinct. NULL is "she never chose", which is the whole
paper; [] is "she unticked every one", which is refused on the screen
where she can still fix it, rather than rendered as a blank exam.
Two things the Flow JSON only told us when Meta was asked. `init-value` is
not allowed on a CheckboxGroup — preselection is `init-values` on the Form,
keyed by component name — and a screen may not route to itself, so REVIEW
routes only to SUBMITTED and re-renders come back as data_exchange. Both
were found by uploading to a throwaway draft Flow and reading the errors;
the file now validates with zero of them.
One latent bug found by the tests rather than by reading: this module's job
entry point is `async function process`, and that declaration shadows the
global across the whole file, so `process.env` here is the job argument's
undefined `.env`. The offer would have been a silent no-op in production.
Gate: 206 tests pass across 11 assessment suites. `npm test` exits 1 on
this branch and exits 1 identically on a clean origin/develop — same 16
suites, same 34 offender files — so nothing here is new debt.
Closes: bd-60023
This repo is public and the source-hygiene guard forbids bd-/BUG-/FEAT- references in shipped code. Two comments in the assessment services carried one. They predate this branch, but the edit above moved one of them onto a new line, which the guard reads offender-by-offender and so counts as new. Removing the refs is the fix rather than widening the guard's allowlist: the sentences say the same thing without them, and an allowlist entry would outlive the reason for it.
feat(assessment): she can untick questions and get the paper again (bd-60023)
… floor
Operator items 6, 11, 14. The hero's meta line was name · Grade N · date;
a transcript spans several grades, so it is now name · date only. `grade`
stays in the signature, accepted-and-ignored, because the caller is owned
by another lane this round.
Every under-floor selector in the teacher PDF now reads from niete-brand's
shared TYPE_FLOOR (body 18/20.7px, small 14/16.1px, label 13.5/15.5px LTR/
RTL) instead of its own number; the letter-mark diamonds and the card
number grew to fit their bigger glyphs, and the figure column narrowed
268px -> 220px to leave the wider option text room.
Urdu chrome no longer transliterates 'quiz' to 'کوئز' in the eyebrow and
the "what this quiz checks" label — both now carry the Latin word, per
the term-of-record rule already settled in ux-strings.js.
While rendering and reading at 390px (round5/E/pdf/*_phone_sim-*.png):
found the wrongpill chip (the wrong-option label on each miss line) was
truncated by a fixed-width CSS nowrap+ellipsis tuned for the old ~11px
scale, and in the Urdu render it clipped from the wrong side of an
LTR-isolated English phrase inside an RTL box ("…roper fraction" instead
of "Proper fraction…"). Fixed by clamping the label in words before it
reaches the chip and letting it wrap instead.
Two cross-lane test files needed matching one-line updates because they
encoded the exact chrome-string/grade behaviour this change intentionally
drops (transcript-quiz-teacher-template.test.js's کوئز probe re-pointed
per its own comment; transcript-quiz-generate.test.js's chrome and grade
assertions updated to match).
Tests: transcript-quiz-teacher-nograde.test.js (new, red then green),
transcript-quiz-teacher-template.test.js, transcript-quiz-generate.test.js
all green; teacher-artefact-type-floor.test.js's teacher-PDF assertions
pass. Root suite and bot suite hold their baselines.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nd says the same words as the document THE CLASS. classesTaught() reads the distinct classes off the FINISHED sessions and hands them to both surfaces — the PDF and the WhatsApp text fallback — so the two cannot disagree about which class took the quiz. The grouping and the sort are not written twice: both this and classHeading() call one exported normaliseClasses(), because two copies of "what counts as the same class" is exactly the pair that drifts and then contradicts itself on one teacher's report. DEPTH. board and stretch are now asked for as 2-3 detailed sentences — the concrete move, the example that goes on the board, what the children do — and muddled/check/secure as exactly one. The token ceiling went 260 to 700: 260 was sized for three one-line fields, and a truncated JSON body does not fail loudly, it fails as a null parse and she silently loses the whole box. THE CONTRACT IS CHECKED, NOT HOPED FOR. guidanceShape() asserts the keys, the sentence count on the field that carries the depth, and the script — then one retry naming what was wrong, then accept whatever comes back, because a short box beats no box. Two distinct events, guidance_thin and guidance_off_language, so the next investigation starts at the right layer. The script test is proportional in BOTH directions, not scriptOf() != target. Both languages legitimately carry a run of the other: an English quiz taught here names روٹی on the board, and an Urdu one names bulb and circuit in Latin because the prompts now ask for exactly that. A presence test flags both and spends a retry on a correct sentence. THE WORDS. The text fallback carries the document's round-5 chrome verbatim — quiz in Latin, the average with its linker, بچے rather than the masculine-marked طالب علم, and the five guidance labels that name what she does. A teacher who gets the fallback one week and the PDF the next must not have to learn two vocabularies for the same three parts.
…AME hand-off
The teacher menu cut lesson names mid-word ("4 Sep · Fractions and Th"), showed
only the ten most recent lessons with no way past them, and a tap on a sent quiz
offered a bare link where the post-coaching flow sends the pre-send PDF and then
the forwardable message.
ONE FORMAT, EVERY ROW. A WhatsApp list row has two fields — title 24 code
points, description 72 — and a real topic does not fit 24, so the topic lives in
the description always: `date · subject` above, `topic · status` below. Which
field the data goes in is decided by the caps, not by how long that particular
lesson's name happens to be; a length-conditional layout makes the shape of the
row depend on which lesson was taught, and that was rejected once already on the
lesson-plan menu. The row statuses were shortened so the status still fits
beside a 40-code-point topic; when even that will not fit, the topic keeps the
field. Measured over the whole account: 744 row renders, the topic cut zero
times, the status dropped once.
Pagination: nine lessons plus an "Older lessons…" row, the header naming the
range ("Lessons 10–18"), the sessions loaded in bounded chunks instead of one
LIMIT 25 that could never see lesson 26. A page past the end falls back to page
one rather than claiming there are no lessons.
The hand-off moved into its own service so "Resend link" runs the SAME thing the
coaching flow runs — the stored PDF from R2, then the identical forwardable
message. The share code is minted at most once, ever: a resend has no branch
that mints, so a row missing half its pair refuses instead of handing the class
a second link. "Regenerate report" is named for what it does and says it is
recounting; the forced regenerate refetches every session, so a child who
finished after the last report is in the new one.
Urdu keeps its own separator next to a number: in Noto Nastaliq the middot
merges with an adjacent digit and "· 1 نے شروع" read as ten students where one
had started.
Red first: transcript-quiz-rows (18), transcript-quiz-list-actions (18),
transcript-quiz-handoff (10), transcript-quiz-report-refetch (4), plus the
extended list suite. Root quiz suite 946 pass + the 1 known pre-existing
lp-shelf failure (baseline 872+1); bot quiz suite 385 pass, unchanged.
…write chain
Two of lane A's PRs merged into develop within minutes of each other. Each was
green on its own branch and the merge is red, which is the whole reason this
repo runs the suites on develop after a merge rather than only on the branch.
One added tests/quiz/video-quiz-session-order.test.js, whose quiz_sessions stub
enumerates `.select().eq().update()`. The other made handleAnswer write the
counters as `.update().eq().or('total_questions_answered.is.null,…lte.N')`.
Neither branch could see the other, so `.or` was never stubbed and the test
died with `TypeError: … .or is not a function` — which reads as the subject
under test breaking, not as a stub that is a link short.
Stub only; no source change. Both suites on merged develop: bot tests/quiz 436
pass, root tests/quiz 989 pass plus the known pre-existing lp-shelf failure.
The repo is public and the source-hygiene guard scans test comments too.
fix(registration): a name on the row means registered — and a SUCCESS screen says so
…692170 test(quiz): stub the .or link the merged counter-write chain needs
`chapter_key` ('c01', 'p3c01') is unique inside a BOOK. It is not unique inside a
(grade, subject), and three sites keyed on it alone. Wherever two books share a
(grade, subject) their chapters collapsed into one menu row: the first row met
supplied the title and the number, and the lesson count became the SUM of both
books. The teacher tapped one row and got a lesson list from two different books.
Measured on prod and staging — identical, so pre-existing and not from the corpus
copy: 49 colliding (grade, subject, chapter_key) groups, 573 menu rows underneath.
Exactly 4 of the corpus's 53 (grade, subject) pairs are affected, in two shapes:
- grade_9_10_chemistry_experiment is one practicals book listed into BOTH years
via also_grades (V1.3.1), landing on the same subject name as the main
Chemistry textbook — 19 collisions on G9, 13 on G10. G9 Chemistry ch.1 read
"Separating the mixture of sand and water" with the textbook's "Nature of
Chemistry in Science" gone and its 5 lessons pooled with the practicals' 2.
All 19 textbook chapter names were absent from the menu.
- Pakistan Studies ships an _english and an _urdu EDITION under one subject
name, both numbering from c01 — 6 collisions on G10, 11 on G12, so a teacher
saw one edition at random.
The fix, in bot/shared/services/lp612-catalog.service.js:
- buildChapterItems groups on (book_stem, chapter_key), orders book-by-size then
chapter (no interleaving; the textbook above the practicals book), and gives
every row on a multi-book screen a short book tag in its title — "Chemistry ·
Ch 1" / "Experiment · Ch 1" / "English · Ch 1" / "اردو · باب ۱". The tag is
derived from the sibling books themselves (language when they differ, else the
stem's non-shared tokens, else the subject) and never from a map of book names:
this module exists so a new book appears with no deploy and no Flow republish.
- buildSegmentItems takes the book and filters on it. Without this the chapter
row is right and the LESSON list is still pooled — which is why the collision
was 573 rows and not 49. The argument is OPTIONAL: a row already in a
teacher's scrollback carries no book_stem and degrades to today's behaviour
rather than erroring.
- buildSubjectItems counts (book, chapter) pairs, so G9 Chemistry advertises 42
chapters and not 23; and it takes RTL furniture only when EVERY row is Urdu,
not when any row is. The old rule rendered "۲۳ chapters" — an Urdu digit
beside an English noun, which in Nastaliq reads "chapters ۲۳" (bd-t8mbl). 2 of
53 pairs were mixed; the 10 fully-Urdu pairs are unchanged.
- one-lesson chapters read "1 lesson". Splitting the pooled rows apart makes
them common (4 of G9 Chemistry's 42) and the module already carried a `plural`
helper it never called.
NO FLOW JSON CHANGE. book_stem rides in the row's own on-click-action.payload,
which the live published staging Flow declares as a bare {"type":"object"} with no
properties — every key this lane already relies on (step, grade, subject,
chapter_key, page) is equally undeclared and round-trips today. Verified against
the published asset, not the repo file; the two are byte-identical.
The row id stays the bare chapter_key on the 49 single-book pairs and becomes
book_stem::chapter_key only where two books would otherwise both claim c01 — the
same condition that puts a tag on the row. Nothing routes on the id.
scripts/lp612-menu-render-proof.js matched source rows on `r.chapter_key === it.id`
and would have silently matched nothing on those four subjects; it now matches on
the payload.
Tests: tests/lp612/chapter-book-collision.test.js — 27 tests driven from 650
VERBATIM staging rows (the 7 books involved plus a fully-Urdu single-book control),
through a Supabase fake that really applies .eq() and PostgREST .or(). 21 fail on
clean develop; the 5 that pass there are controls that must pass on both sides.
Four existing assertions updated for the widened contract, each with its reason.
Delta gate: the repo gate run on a fresh worktree of develop db914e3 and on this
branch produces byte-identical regression blocks (260 lines each) — zero new
failing suites, tests or offenders. lp612 suite 1141->1168 passing, same single
pre-existing failure (brief-field-coverage).
Closes: bd-oak77.5
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
fix(lp612): a chapter row belongs to ONE book, not to a chapter_key (bd-oak77.5)
The child finishes, and we offer her a friend invite, a binge round, a class
share, a survey. Until now nothing recorded that an offer was SHOWN and nothing
recorded a decline, so the only measurable half of the funnel was the yes.
Every offer now emits video_quiz.offer_shown{kind,sessionId,quizId,source,
language} and every tap video_quiz.offer_answered{kind,choice,...}, with
kind in quiz|invite|share|binge|feedback and choice a stable token rather than
a button title. Point events beside them: scorecard_sent (with the fallback
verdict), binge_started / binge_unavailable{reason}, binge_video_picked,
feedback_answered. Ids, enums and counts only — no name, no phone, no free text.
The feedback offer is logged at SEND time, not at schedule time: the survey is
queued 30 s ahead and the send can fail, and an offer nobody received is not an
offer shown. The share offer carries the retry's verdict for the same reason.
Additive throughout: new logEvent lines, plus optional sessionId/quizId
parameters carried into the offers and stored in the Redis ctx each already
writes, so an offer minted before this deploy still answers cleanly.
docs/quiz-telemetry.md holds the event table and the funnel queries.
Also recovered docs/flows/student-join-flow.json — the join Flow was published
on a WABA and its JSON existed nowhere in the repo, so no second deployment
could ever be given the same screen. scripts/publish-student-join-flow.py
publishes it (no endpoint_uri: it is a navigate Flow whose reply arrives as an
nfm_reply).
Measured on staging today with a ten-phone burst against one share code. The
ten inbound QUIZ-<code> texts were answered 200 in 2.4-3.1 s. Every one of the
join-Flow replies that followed got NO HTTP response at all, and a single
controlled retry was still unanswered after 180 s — while the bot had created
that child's session one second in and already sent her three messages.
The cause is structural, not local. whatsapp-bot.js:1602 (the vqjoin: branch)
and :1618 (the picture-answer branch) end in a bare `return`, and a return
inside app.post('/webhook') leaves the route without calling res.send. About
twenty other returns in the same handler — the button_reply and list_reply
chains for training, capstone, commitment cards, lp612 feedback, attendance,
media targets, conversation resume — have exactly the same shape.
The message-id dedupe upstream (SessionService.isProcessed) means a Meta
re-delivery does not do the work twice, so this is not duplicated sessions. It
is a socket held open on every interactive tap, and a webhook endpoint Meta
sees as failing to answer. It also retires the round-4 reading of bd-mg9c7.55:
the driver's 30 s timeouts were never 'the driver's own wait'.
The fix is one ack, sent as soon as the message is accepted for processing,
through a res.headersSent-guarded helper that the thirteen pre-existing sends
now go through unchanged. From that point every branch may return freely, and
a slow handler — a quiz start is about six paced sends — can no longer hold
the socket past Meta's patience.
Red first: tests/quiz/webhook-ack.test.js boots the real Express route, POSTs
a vqjoin: and a vq: nfm_reply, and asserts on the RESPONSE. Before the fix both
handlers ran and the response never arrived (5 s deadline, status null). A
source-level assertion could not have caught this — every branch reads
correctly; it is the absence of a call that is the bug.
No delta on the twelve suites that load whatsapp-bot.js: 5 failed / 7 passed,
93 passed / 2 failed, identical with and without this change.
Closes: bd-mg9c7.74
Round 4 closed bd-mg9c7.55 at 'measure before anyone claims it is fast', and nothing timed a single send. sendPhase — the one place every quiz send passes through — now emits video_quiz.phase_sent with the wall time DECOMPOSED into msSending (the WhatsApp calls), msThrottled (our per-recipient throttle) and msGaps (our own deliberate 700/1200 ms pacing), so a slow phase can be pinned on Meta, on our throttle, or on us. handleAnswer emits video_quiz.answer_latency with msToFeedback, msToNextQuestion, the fixed 1200 ms pause and the media kind. What the constants already say, now measurable: a three-to-four message question phase spends 2.5-3.1 s in self-imposed pacing before any round trip, and the 1200 ms pause sits on top of it. The two deliberate delays, not Meta, are the levers. Nothing here changes them — this round measures. The 23505 reconcile branch emits no answer_latency: that tap delivered no verdict, and counting it would read as a normal graded answer. A finished:true row carries the whole completion chain in msToNextQuestion, so the doc's query filters it out rather than letting the scorecard render skew a p95. Behaviour unchanged: same messages, same order, same gaps, same return value.
…full rewrite (bd-ga7xz)
BREAKDOWN.md §4 lever 2: every revision round re-emits the entire ~7,900-token
lp_doc (R²=0.911 against output tokens alone) -- a round that exists to fix
one page rewrites the whole lesson. This adds a targeted-revision path behind
LP612_TARGETED_REVISION (default FALSE):
- the revision prompt (targeted:true) asks for a pointer -> whole-subtree
REPLACEMENT MAP ({"replace": {"<pointer>": <value>, ...}}), constrained to
a bounded, doc-derived address space (deriveAllowedPointers: one pointer
per top-level section, one per page2 key, one per other top-level key --
never a hardcoded schema guess);
- an escape hatch ({"full": {...}}) for structural fixes, named explicitly
in the prompt with the criterion for when to use it;
- applyReplacements(doc, replace) merges onto a DEEP CLONE, all-or-nothing:
every pointer must be in the allowed list, resolve to an EXISTING
location, and match the existing value's JSON kind (object/array/
primitive) -- one bad pointer rejects the WHOLE patch;
- a rejected or unparseable reply falls back, within the SAME round, to the
existing full-rewrite prompt exactly once -- a bad patch costs at most
one extra call, never the lesson;
- everything downstream (applyVideo/sanitizeUnknownTopLevel/sanitizeOverlay/
stripOverlay/sanitizeSequence, runGates, notWorseVisual) is UNCHANGED --
a patched candidate is gated exactly like a rewritten one, which is the
whole safety argument for this change.
Shape chosen on measured evidence, not preference: sanitizeOverlay's own
comment block records this model mis-pointing fine-grained JSON Pointers at
a real rate (8 of 55 ur_overlay pointers on one document addressed blocks it
had not written, bd-vnyuw) -- coarse, named whole-subtree replacement removes
that failure mode by construction instead of validating around it.
Scope: only authorLessonPlan()'s revision ladder reads the flag.
reviseLessonPlan() (the teacher-edit lane) is untouched and always asks for
a full rewrite -- a deliberate choice, not an oversight (different product
surface, different round budget).
Telemetry for the A/B (bd-ga7xz + bd-5w13w's promptSha groundwork): patchMode
('replace'|'full'|'fallback'), pointersReplaced, and the round's
completion_tokens land on the existing "lp612 author revision round" log
line and on logEvent('lp612.author.revision_round', ...). A rejected patch
also emits logEvent('lp612.author.patch_rejected', {reason, pointers}).
Flag: LP612_TARGETED_REVISION (bot/shared/config/lp612-flags.js,
isLp612TargetedRevisionEnabled(), explicit-boolean convention, default
false). authorLessonPlan({ targetedRevision }) overrides it per call for the
A/B harness.
Tests: tests/lp612/targeted-revision.test.js -- unit coverage on the pure
merge primitives (deriveAllowedPointers/applyReplacements/parseTargetedReply)
plus the six ladder-level scenarios from the brief, driven through the real
authorLessonPlan() with the LLM doubled at llm-client (never the module under
test). All 24 assertions proved RED against unmodified develop (missing
exports / missing option / missing telemetry) before this diff landed.
Closes: bd-ga7xz
Closes a genuine new offender the delta gate caught: env-template-completeness.test.js (already-red on develop) grew a NEW item — the new flag itself — because it wasn't listed in .env.template. Documented alongside LP612_AUTHOR_ROUNDS, following the existing convention for this file's lp612 flags.
lp612: targeted revision — pointer->subtree replacement instead of a full rewrite (bd-ga7xz)
…e menu (#681) The operator, 2026-09-06: "turn off the free flow lesson plan generation via gamma on prod, and route all lesson plan requests to this menu." Most of that ask is already true on develop — the Gamma strip retired freeform generation and the one-door change funnelled every entry point into openLpBrowseFlow(). Verified against origin/main: prod still runs the Gamma generation (handleLessonPlanRequest queues it) and has neither lp-browse-entry.service.js nor lp612-flags.js. So the promotion IS the Gamma cut. This closes the two leaks the promotion does not, behind one new flag. LP_612_ROUTE_ALL (default false, narrows LP_612_ENABLED): - LEAK 1: tryCurriculumLessonPlanServe reached the OLD 6-12 Oxbridge picker from three call sites, all upstream of the menu — the pre-classifier early intercept (fires on EVERY inbound text), the lesson_plan intent, and the awaiting_topic reply. The guard is the first thing that function does, ahead of the region_features lookup, so a routing promise does not depend on a DB row. Returning false is the mechanism: all three callers fall through to the menu. - LEAK 2: the Flow endpoint fell back from an empty 6-12 catalogue to Oxbridge subjects. Suppressed; she gets the honest "nothing for this class yet". - She typed a topic, so one short line first (lp612RouteRedirect, en/ur, from the one catalogue) before the grade picker. Text and voice doors both. LP_612_ROUTE_K5 (default false): the switch for "does all include K-5". The 6-12 menu cannot serve K-5, and K-5 already reaches the same Flow through the same grade picker, so the default leaves her exactly where she is. Built so the product owner's answer is a flag, not a rebuild. lp612ServesGrade() is the one definition of which grades the corpus claims, shared by router and endpoint. The cutover moment, for the teacher already waiting: lp_variant='gamma_freeform' is being written on prod right now, so at the deploy some teachers have a Gamma lesson in flight. The worker already drained those messages rather than looping — what it sent was a dead end asking her to type "menu". She is now handed the menu, in the language frozen into the job payload, with the same one line; the old apology still fires if no Flow is provisioned, so she is never left silent. Also fixed, both live entry points found while mapping: - The broadcast template button "Lesson Plans & Assessment" arrives as text (Meta strips the payload). The one-door narrowing dropped it and template-button-fallthrough.test.js has been red on develop ever since. Matched exactly again, as a label we publish; BARE is unchanged. - text-message-lp-keyword.test.js: 6 failing on clean develop, all stale. Two were source-text greps pinning how a line was TYPED, not what it does; two demanded pre-narrowing sentence matches; one had a name and an assertion that contradicted each other. Now 27/27. Copy: lp612Preparing and lp612Restarted no longer quote "5-6 minutes" — the number had drifted out of true (measured ~6 min EN, ~9 min UR), and quoting a wrong number is the defect honest-eta.test.js exists to catch. They promise the follow-up the code actually sends. Those tests are inverted, not deleted, so nothing can put a number back. TDD red-first throughout: 8/13 red on the router suite before the change, and the endpoint suite caught a real bug — isLp612RouteK5() consulted without the LP_612_ENABLED gate would have taken the K-5 corpus away while the master flag was off. 26 new tests. Delta gate against a fresh worktree of develop @ db914e3: every failing-suite and offender category identical, plus one suite green. Refs: bd-oak77.4, parent bd-oak77. Co-authored-by: Haroon Yasin <hyasin132@gmail.com>
Lane E — teacher artefacts: the type floor, the derived class, honest Urdu, and a reteach box she can read
R5 lane F — /quiz: every topic in full, pagination, and a resend that is the SAME hand-off
The engine's twenty diagram types are the 6-12 lesson-plan roster: they draw
quantity, structure and process. A grade 1 phonics, spelling, counting, time,
money, pattern or matching lesson could reach none of them, so the author prompt
told a young class's quiz to leave "figure" null and a picture question at that
age was impossible.
Eight new engine types, drawn from the engine's own primitives over a vendored
open-licence pictogram set:
word_blank a pictogram and its word with letters hidden — phonics,
spelling, missing letter. An Urdu word is drawn as separate
letter tiles, never as a joined word with an underscore in it:
Nastaliq joins, so removing a letter reshapes its neighbours
and shows the child a word that is not the word.
count_objects n things to count, two rows to compare, or equal rings to share
count_frame a ten-frame, or tally gates of five
clock an analogue face whose hour hand is at (h + m/60) x 30 degrees,
so 3:30 puts it halfway between 3 and 4
pattern a repeating sequence with one "?" slot
match two lettered/numbered columns with NOTHING joined
money coins as labelled discs, notes as labelled rectangles
compare_size bars or a balance — longer/shorter, heavier/lighter
Pictograms: 207 line-art glyphs from OpenMoji 15.0.0 (CC BY-SA 4.0), chosen by
intersecting the concrete nouns of two curriculum segmentations with what
OpenMoji actually has. The licence and attribution ship with them and the credit
is written into every figure's own <desc>, because a figure is delivered as a
bare PNG with no page around it. A name the set lacks fails validation with the
roster attached — never a blank box.
Wiring: the eight join ALLOWED_TYPES; the author sees them only for a grade 1-5
lesson (a grade 9 chemistry quiz has no use for a ten-frame, and every type in
the prompt is one more shape the model can reach for wrongly); a grade 1-5
lesson is now DRAWABLE IN EVERY SUBJECT, because these are language types before
they are anything else.
Also in this change, both found by measuring rather than by reading:
* THE PHONE FONT SCALE IS NOW A CEILING, NOT A SETTING. The per-type scale was
chosen by sweeping one spec per type. Swept against every example each type
ships, EIGHT correct, engine-authored drawings are rejected by the quiz lane
today purely because of their type's scale — a rejected figure is a dropped
question. renderFigureSvg now steps down the ladder until the drawing is
clean, so a sparse spec keeps the big type and a dense one gets a smaller one
instead of the bin. Each step-down is logged.
* THE ENGINE'S OWN WORDS NOW FOLLOW THE FIGURE'S LANGUAGE. atom and ray_diagram
write a caption when the author gives none, and those were English whatever
lang said. Fixing it surfaced a second defect: ray_diagram's halo() computed
its own label plate with arithmetic that does not match the Urdu text box, so
the plate was narrower than the label and every Urdu ray diagram collided six
ways the moment its labels stopped being English. It now uses plateText.
Tests: 90 new, every one through the quiz lane's own renderFigureSvg rather than
the engine directly, so a red run proves the vendored copy is what changed. Red
confirmed for each group by restoring the pre-patch files and re-running.
…r write The counter write now ends .update().eq().or(...) — this stub's chain stopped being awaitable one link short of it after the rebase.
…roup Flow
A quiz question whose answer is a SET had nowhere to live: WhatsApp reply
buttons and list rows are single-select, so a question with two right answers
could only be asked as if it had one. It is now authored, stored, sent, scored
and printed as a set.
author a question may carry answer_mode:"multi" + correct_indices; at most
two per quiz, never question 1, 3 or 4 options, each option capped
at the CheckboxGroup title cap. The contract only reaches the prompt
when the Flow is configured — asking for a question we cannot
deliver is worse than not asking.
store correct_option holds the shuffled set as "A,C" and media.answer_mode
marks the row. No schema change: asserted against the live database
(correct_option is TEXT with no A/B/C constraint, option_d exists)
rather than read off the schema file, which still carries the stale
CHECK.
send a static one-screen Flow with a CheckboxGroup, everything the child
reads supplied per-send as screen data — a Flow asset is one per
WABA and cannot be re-rendered per language. The card or figure is
the Flow's image header. With the Flow id unset the question
degrades to the ordinary picker carrying a line that says more than
one answer is right, with a logged event. It never blanks.
score exact set equality; the verdict opens with the marker, names what
was missed and what did not belong, and keeps the author's own
per-option words. A double submit takes the existing reconcile.
print the teacher's paper marks EVERY correct option (it kept only the
first) and carries the cue; the class report resolves a set to both
option texts instead of null.
The Flow never learns how many answers are right: no min/max-selected-items,
because the count is the answer and Meta renders an untranslatable English
"Choose 1 to N options." line from them (seen in the live preview).
The cue is written by code on every surface, in both languages, and the author
is forbidden to write it into the stem — a prompt rule the model follows most
of the time would leave one child in twenty facing a set question that looks
single.
Red first: 26 root + 14 bot tests written and run against this branch before
the wiring existed (17 and 8 red respectively).
Suites: root tests/quiz 872->958 pass (+86), same one known pre-existing
failure; bot tests/quiz 385->399 pass.
fix(llm-client): explicit per-call timeout + maxRetries, env-overridable
The repo is public and the source-hygiene guard scans bot/ for them. The comments say the same thing without the reference.
quiz(observability): the post-quiz funnel, tap latency, and the webhook that was not answering children
feat(quiz): early-years figure types — a picture question for grades 1-5
feat(quiz): select-all-that-apply questions, delivered as a CheckboxGroup Flow
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DO NOT MERGE without a fresh operator "go"
This branch is the vehicle for the lp612 (6–12 Lesson Plans) production go-live. It is built, gated and parked. Merging it ships everything on
developto production, not only lp612.Merge:
origin/maine7b10e7←origin/developdb914e3(merge basef36de7d). develop was 904 ahead, main 150 ahead.Gate — delta method, three trees
Ran
node tests/run.json fresh worktrees oforigin/main,origin/developand this branch.origin/maine7b10e7origin/developdb914e3develop's (set difference empty in both directions), and its failing-test names match develop's exactly.mainare green here:tests/coaching/fico-framework.test.js,tests/flow-response/endpoint-flow-routing.test.js.mainare red here, both inherited fromdevelop(pre-existing debt, not created by this merge):tests/handlers/text-message-lp-keyword.test.js(6 tests — the LP keyword intercept; the 02_routing lane owns it) andtests/setup/language-single-source.test.js(1 test — "no inline en/ur clamp left in bot/").require()s here vs 9 onmain— 0 new, 1 removed.Conflicts
27 files. Per-file rationale, both sides' intent and the owning lane are in the lane folder's
CONFLICTS.md. Summary: 22 resolved todevelop(it demonstrably supersedes — FICO v4, the lp612 worker case, the uptake loop, the cassette front, the registration predicate), 1 tomain(the data-standards Slack step, whichdevelopnever had), 4 as a union of both sides.Verified explicitly, not assumed:
docs/flows/observe-visit-v2.json— the two sides are semantically identical (jq -Sdiff = 0 lines); the conflict was indentation only. No prod Flow asset content changes.bot/scripts/setup/flow-configs.jsauto-merged to 17 entries.main's 18th, Pic-to-LP Confirm, is gone becausedevelopcommit54f90df(bd-2540 Phase 1b) deliberately stripped the teacher-triggerable Gamma pic-to-LP path. That is intended, and it is a user-visible production change.@google/generative-aileavesbot/package.jsonfor the same reason (df37a8b, bd-2540 Phase 1c, Gemini vocabulary-image path stripped from reading assessment). No dangling require remains.One deliberate deviation from a straight merge
infrastructure/supabase/migrations/V1.2.1__schools_declared_columns.sqlandV1.2.2__teacher_roster_admin.sqlare removed in the second commit.main's ownV1.2.3__leader_roster_audit.sqlstates verbatim that V1.2.2 "can no longer be applied as written" and that "production goes to 1.2.3 without it. That gap is deliberate."infrastructure/scripts/migrate.jsapplies every pending migration in version order with no file selection, so carrying those files ontomainwould let the next runner apply them to prod. Nothing intests/,bot/,dashboard/ordocs/references either file. Revert that commit to take the straight merge instead.Before this is merged
V1.2.8__lp612_runtime_lesson_plans…V1.3.8__lp612_over_timearrive as files and must be applied to prod by hand first, effect-asserted per file (01_prod_db lane).main only accepts stagingcheck will be red on this PR.mainhas no required status checks configured, so it is a red check rather than a merge blocker — the same route PRs Assessment Generator → production (feature only, all flags off) #624–fix(assessment): the edit list to production — Done is a button, the rows have their text back #675 took.developupdate today (02_routing). This branch will be re-merged before any go.Update — re-merge #1, 2026-09-06 (
12e32fc)git merge origin/developat59d4675, 0 conflicts. Still DRAFT, still DO NOT MERGE.The A2 conflict resolution turned out to be a P0, and is now fixed at source. The pre-go check this PR asked for came back bad: on NIETE prod, 7,685 users carry a
first_nameand only 440 carryregistration_completed; 7,245 have it false and 4,145 of those messaged the bot in the last 30 days (3,951 teachers).registration_stateis no fallback — 'unregistered' on 7,684 of the 7,685, including 439 of the 440 whose flag is true. Develop's predicate would have sent 4,145 active teachers back into registration.Fixed on
developin #686:bot/shared/utils/registration-status.jsis now the one definition (a completed run OR a name already on the row), read by both gates intext-message.handler.js(:1780,:2278); andregistration-endpoint.jsnow marks the account complete at bothSUCCESSbranches, which is the second fault — it was telling teachers "Your registration is complete" without ever writing the flag, hence a 76.7% completion rate over the last 7 days. 9 root-tree tests, 5 of them red before the change.Gate after the re-merge, two full runs: develop
59d467532 failing suites; promotion 32 and 32. Merge-introduced failures: 0. Every difference resolved to a flake —render-slow-telemetrypasses 4/4 in isolation on both trees;portal-capstone-submit,portal-grand-quizandsqs-cancel-by-groupare ontests/BASELINE.md's own quarantine list.Still owed before a go, unchanged: FICO v3 → v4 is not flag-gated (every new prod coaching score changes scale); Pic-to-LP is removed by the vehicle; the 11 lp612 migrations must be hand-applied first. And
BACKFILL.sqlforregistration_completedis written in the lane folder and not run — the bot is correct without it, but the column, the coach portal's/teacherslist (440 of 7,685 today) and every adoption metric are not.Update — re-merge #2, 2026-09-06 (
1a01a20) — all lanes now on the branchgit merge origin/developata33f74a, 0 conflicts. Still DRAFT, still DO NOT MERGE without a fresh operator go.Every lane checked physically on the branch, not inferred from a PR list:
bot/shared/services/lp-browse-entry.service.js;bot/shared/config/lp612-flags.jsexportsisLp612RouteAllandisLp612RouteK5;text-message.handler.js:36requires them, called at:121and:2722.llm-client.jscarries an explicit per-calltimeout+maxRetries(the SDK defaults it replaces were 600000ms / 3 attempts);tests/unit/llm-client-timeout-{wiring,behavior}.test.js.bot/shared/utils/registration-status.js;isRegistered(user)at 2 call sites;markRegistrationCompleteat 3 references.pakistan-lp-endpoint.js,sqs-visibility-heartbeat.js,bot/vendor/lp-v9.main-source-branch.yml,V1.2.3__leader_roster_audit.sql. Deliberately absent —V1.2.1,V1.2.2.Gate — three fresh trees:
maine7b10e728 failing suites /developa33f74a31, 31 / promotion 32, 31. Merge-introduced failures: 0 — the single run-1 difference wastests/queue/sqs-cancel-by-group.test.js, the first entry ontests/BASELINE.md's own quarantine list, which passes 3/3 in isolation here and did not recur; run 2's failing-suite set matchesdevelop's exactly in both directions.tests/handlers/text-message-lp-keyword.test.jsis now GREEN — the routing lane cleared it, so the only suite green onmainand red here istests/setup/language-single-source.test.js.Migrations: no new migration arrived — the eleven lp612 files are unchanged. But the sweep found three SQL files on a path
infrastructure/scripts/migrate.jsdoes not scan:bot/database/migrations/{transcript_quiz,roster_backstop_uniques,create_calls_tables}.sql. None blocks the go —transcript_quiz.sqlis two indexes behindTRANSCRIPT_QUIZ_ENABLED,create_calls_tables.sqlserves a service prod does not run — butroster_backstop_uniques.sqlguards live roster/attendance writes and is aCREATE UNIQUE INDEXthat will abort on existing duplicates, so count them before applying.Still owed before a go: FICO v3 → v4 is not flag-gated (every new prod coaching score changes scale); Pic-to-LP is removed by the vehicle; the 11 lp612 migrations must be hand-applied first;
BACKFILL.sqlis written and not run.