chore: re-sync main (prod) into develop (staging) — 14 commits, 1 conflict - #306
Merged
Conversation
…e one
NIETE offers Urdu first but STORES English by default, and has since the offer was
introduced. `LANGUAGE_OFFER[0]` is 'ur' and `offerDefaultLanguage()` returns 'ur',
while `users.preferred_language` carries a Postgres column DEFAULT of 'en'. A
teacher who opens the picker sees Urdu at the top; a teacher who never opens it
gets English. On prod that is 9,389 of 9,508 teachers on English, 97.6% of whom
registered before the offer existed. Of the 119 who have expressed a preference,
117 chose Urdu.
Note what this means for the obvious fix: changing `DEFAULT_LANGUAGE` in
language-cache.js is a NO-OP. Every row holds a non-null value, so the JS floor
never fires for a real teacher. The floor is the column default, not the constant.
WHY A SCRIPT RATHER THAN ONE UPDATE STATEMENT
1. The cache. `user:language:{id}` and `user:language_locked:{id}` hold a 24h
TTL and a SQL write invalidates neither. `setUserLanguage()` writes through
to both, and is the ONE WRITER the language protocol requires.
2. `isOffered()` validation lives inside that writer.
3. Per-row accounting, so a partial run is visible and resumable rather than
assumed complete.
THE ONE IRREVERSIBLE MISTAKE, AND HOW IT IS GUARDED
`language_locked` is the ONLY thing separating "she chose English" from "we
defaulted her to English". So this touches only `language_locked != true`, and
passes `lockLanguage = false` EXPLICITLY — the writer defaults it to TRUE. Had it
set the lock, a backfilled 'ur' would be indistinguishable from a chosen 'ur'
forever, no future default change could move those rows, and rollback would be
impossible.
That argument was worth nothing until it was tested. Mutation showed that dropping
the `false` left all 19 tests GREEN — the single unrecoverable error available here
was completely unasserted. The writer call is now an injectable `flipOne()` whose
third argument is asserted directly; dropping it or setting it true each fail two
tests.
THE CACHE IS ON A PRIVATE NETWORK
`REDIS_URL` points at `redis.railway.internal`. Verified that `railway run`
injects the variables but does NOT join that network, so from outside Railway the
variable reads as "present" while being unreachable. With Redis down,
`redisService.set()` returns false SILENTLY and `setUserLanguage()` ignores that
return value — so it reports SUCCESS while the cache is never written.
Three states, no fourth: cache ready -> write-through; unreachable -> REFUSE;
unreachable + explicit `--accept-cache-decay` -> proceed in a mode recorded as
'decay' in the snapshot, so a decayed run can never be mistaken for a clean one.
The flag cannot downgrade a healthy cache, and a test asserts no state proceeds
silently.
Decay exposure is MEASURED per environment at run time, never quoted from an
earlier investigation. The cache is populated lazily on read, so only teachers
active inside the TTL hold an entry: exposure is the intersection of the rows
being changed with the recently-active set, not the active total. On prod that
measured 651 of 9,389 (6.93%); the other ~8,738 have no entry and change
immediately. Those 651 keep the language they already have for at most 24h — a
delayed improvement, not a regression.
SAFETY
- dry run by default; `--live` required; prod additionally requires `--yes`
- `--env` is mandatory with no default, because an implicit prod target is how
the wrong database gets written
- project ref asserted BEFORE any client is constructed, since a worktree seeded
from another repo's .env points at a DIFFERENT production database
- pre-change snapshot written before the first write; rollback skips anyone who
has chosen a language since, because her lock outranks our revert
- idempotent and resumable — a second run selects nothing
- verifies by RE-READING the database: locked counts unmoved, no off-offer row
touched, zero candidates left
Off-offer languages are excluded, not "corrected" — invariant 7 says grandfather
them. Snapshots are gitignored: they hold user UUIDs and are run evidence, not
source.
Known side effect: `setUserLanguage()` also writes `updated_at`, so this bumps it
on every affected row. Anything reading that as recent teacher activity will see
a spike on the migration date.
NOT INCLUDED, DELIBERATELY
`ALTER TABLE users ALTER COLUMN preferred_language SET DEFAULT 'ur'`. Without it
every new registration returns to English and this regrows, so it belongs in the
same window as the backfill — but it is a schema change and gets its own review.
The `DEFAULT_LANGUAGE` constant also still reads 'en'; it only fires on a DB error
or a missing row, and leaving it disagreeing with the column default is how the
"fallbacks disagree" defect happened before.
Verified: 26 tests, every guard mutation-tested. Applied to staging (9,026 rows,
0 failures). Prod deliberately untouched and confirmed so by querying it for this
migration's signature (preferred_language='ur' AND language_locked=false): 0 rows.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two defects in this branch's own tooling, found by actually running the repo suite
rather than only the standalone runner:
- The tests were a hand-rolled runner with `process.exit()`. They matched
`*.test.js`, so jest collected them and FAILED the suite — this branch added a
failing suite while its own tests reported 26/26 green. Converted to
describe/it with expect-based helpers, which is also the repo convention, so CI
now actually executes them.
- The CLI sat behind a top-level `if (require.main !== module) return;`. Node
permits that inside the CommonJS wrapper; Babel — which jest uses to transform
the module when the tests require it — rejects it as "'return' outside of
function". The guard parsed fine under `node` and made the module impossible to
load from a test. The CLI is now wrapped in `main()`.
Both were invisible to the standalone runner and only appeared when the suite ran
the way CI runs it.
Review of this branch found a real defect. Both paginated reads used `.range()` with no ORDER BY. `.range()` is LIMIT/OFFSET, and Postgres guarantees no stable row order without an ORDER BY — so with registrations landing continuously, rows can shift between pages and be silently skipped or double-counted across a 9,500-row scan. A skipped row means a teacher quietly not migrated, with nothing in the output to say so. Both scans now order by a stable key (`users.id`, the primary key; `created_at` for the activity scan). The ordering alone is not proof, so `fetchAllUsers` also compares its row count against an exact server-side count and ABORTS on a mismatch rather than migrating a partial set. Verified on staging: the scan reports all 9,067 rows and 0 remaining candidates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This deployment OFFERS Urdu first but STORED English by default, and had done since
the offer was introduced. `LANGUAGE_OFFER[0]` is 'ur' and `offerDefaultLanguage()`
returns 'ur', while the column default was 'en'. A teacher who opened the picker saw
Urdu at the top; a teacher who never opened it was stored as English. Both behaved
exactly as coded.
At the time of writing: 9,391 of 9,510 teachers on English, 97.6% of whom registered
before the offer existed. Of the 119 who had expressed a preference, 117 chose Urdu.
The JavaScript constant is NOT the fix. `DEFAULT_LANGUAGE` in language-cache.js also
reads 'en', and changing it is a no-op for real teachers: every row holds a non-null
value, so `preferred_language || DEFAULT_LANGUAGE` never fires. The floor that
decides a new teacher's language is this column default. That constant is left alone
deliberately — it governs failure behaviour (no row, failed read) and is a separate
decision.
Scope: NEW rows only. A DEFAULT change does not touch existing rows; those are
migrated by scripts/language-floor-flip.js, which touches only teachers who never
chose and never sets the lock. The two belong in the SAME window — without this
default, every new registration returns to English and the backfilled population
regrows immediately.
Safety: no table rewrite. SET DEFAULT is a catalog-only change on an existing
nullable column. No CHECK constraint or enum governs the column (verified against
pg_constraint), so 'ur' is accepted. Rollback is the same statement with 'en'.
Verified on staging, and not by reading the DDL back:
- information_schema reports the default as 'ur'
- a REAL insert of a new row produced preferred_language='ur', language_locked=false
- re-applying this file when the default is already 'ur' succeeds, so it is
idempotent
- the probe row was deleted; zero remain
- tests/setup failure set is byte-identical with and without this file (13 suites,
all pre-existing on develop), so it introduces nothing
One gotcha for whoever probes this next: phone_number is varchar(20), so a long
placeholder value fails the insert rather than the default.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
prod: preferred_language default en → ur + backfill for teachers who never chose
…-resolver observe: one owner for language — the resolver (bd-04m67) (cherry picked from commit df03e75)
observe: one owner for language — report + notes move onto the resolver, toggle deleted (bd-dy7hs, bd-c3uq9) (cherry picked from commit dc78cfe)
observe: raw audio must not become an observation (bd-jrxo3) (cherry picked from commit 9f0f7af)
observe: coach_directory — resolve a coach to her work email once (bd-o98ji) (cherry picked from commit 64c844a)
observe: calendar invites for scheduled observations, default off (bd-dk6hy) (cherry picked from commit 72f80fc)
prod: HITL integrity, one language owner, coach directory + calendar (dark) — 5 phases
…s not have — plus success telemetry Found while switching the feature on. Two things stood between "built" and "actually delivers an invite". 1. SCOPE. google-calendar.client.js asked for https://www.googleapis.com/auth/calendar.events. That scope is NOT in this service account's domain-wide delegation grant, so minting a token fails outright with `unauthorized_client` — and because every calendar call is deliberately non-blocking, EVERY invite would have failed in total silence. Nothing in a coach's calendar, nothing obvious in the logs. Verified per scope against Google with the real key before go-live: GRANTED drive · documents · spreadsheets · calendar DENIED calendar.events The broader `calendar` scope is granted and does the same job — proven by creating, patching and deleting a real event on rumi@hellorumi.ai. So this is a one-line fix, NOT a Workspace Admin request. The comment above the constant names the denied scope on purpose, so nobody "tightens" it back. 2. TELEMETRY (operator request). The service logged only failures, so a working invite left no trace and "did she actually get it?" could only be answered by opening her calendar. Every lifecycle op now emits a success line — `invite sent` / `invite moved` / `invite removed` — carrying the schedule id, the Google event id, the recipient, and (on create) the teacher and date. The create log lives in the shared _create helper, which both the scheduled and the reschedule-with-no-event paths run through. Failures stay logged and stay non-blocking; a skip still logs nothing, so the success line means an invite really went. 10 tests, red first. Suite: 15 failing before and after, zero regressions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 768cba4)
prod: calendar asked for an ungranted scope — every invite would have failed silently
… container has no key file The last thing between "configured" and "an invite actually arrives". google-calendar.client.js read the service-account key from GOOGLE_SERVICE_ACCOUNT_PATH — a FILE PATH. Railway containers have no such file, and the variable was not set on production at all, so isConfigured() returned false, the gate skipped every schedule, and not one invite was even attempted. Silent, because the whole calendar path is deliberately non-blocking: no error, no event, nothing in a coach's calendar and no obvious reason why. Now ENV FIRST, file second — the convention this project already documents (coos-088): GOOGLE_SERVICE_ACCOUNT_JSON for the cloud, the local file for a workstation. A bad path no longer breaks a working env var. 6 tests, red first, including the exact state prod was in (subject and calendar id set, no key of either kind => not configured). Suite: 15 failing before and after, zero regressions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 6fb5fc9)
prod: calendar key must come from the env — a container has no key file
… converged Follow-up to the bd-43475 back-merge. main had drifted 14 commits ahead again in a few hours. One conflict this time — .gitignore — because the back-merge already absorbed the hard work. Worth recording WHY the drift recurs: `git cherry` shows only 3 of main's 14 commits are genuinely new patches. The other 7 already exist as equivalent patches on develop — the same fix lands on develop AND separately as a prod-* branch merged straight to main. Both branch HEADs were literally the same calendar-key fix arrived at by different routes. Until develop -> main is the only path to prod, the branches will keep re-diverging no matter how often they are synced. .gitignore -> develop: keeps the `.beads` symlink guard (a machine-absolute symlink that breaks every clone if committed) and the bd-2540 working-note ignores. main's side of the hunk was empty; main's other additions auto-merged elsewhere in the file. What main brought that develop did not have (the 3 real patches): - migration V1.1.5: users.preferred_language DEFAULT en -> ur - scripts/language-floor-flip.js: backfill ur for teachers who never chose - bd-dk6hy: Google Calendar invites for observation schedules Verification (identical environments): npm ci root / bot / portal all succeed conversation-state + status 87/87 pass tests/setup conformance 13 fail, same names as baseline tests/portal 297/297 pass portal build + build:app both succeed conflict markers 0 bot lockfile in sync (chart.js present) — NOT main's broken one V1.1.1 + pic-to-LP state preserved (19 migrations, 0 pic-to-lp files) Does NOT change prod. Two prod-side issues remain open and independent of this merge: main's own bot/package-lock.json is out of sync with its package.json (chart.js missing, so `npm ci` cannot succeed on main), and main's migration ledger skips V1.1.1, V1.1.3 and V1.1.4 while having applied V1.1.5-V1.1.7. Closes: bd-43478
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
3 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.
Why
Follow-up to the bd-43475 back-merge.
maindrifted 14 commits ahead again within hours. One conflict this time (.gitignore) — the back-merge already absorbed the hard work.The drift is structural, not a promotion-speed problem
git cherryshows only 3 of main's 14 commits are genuinely new patches. The other 7 already exist as equivalent patches on develop — the same fix lands ondevelopand separately as aprod-*branch merged straight tomain. Both branch HEADs were literally the same calendar-key fix, arrived at by different routes.Until
develop → mainis the only path to prod, the branches will keep re-diverging however often they're synced.What main actually brought (the 3 real patches)
V1.1.5—users.preferred_languageDEFAULT en → urscripts/language-floor-flip.js— backfillurfor teachers who never choseResolution
.gitignore→ develop: keeps the.beadssymlink guard (a machine-absolute symlink that breaks every clone if committed) and the bd-2540 working-note ignores. main's side of the hunk was empty; main's other additions auto-merged elsewhere in the file.Verification (identical environments)
npm ciroot / bot / portalconversation-state+statustests/setupconformancetests/portalbuild+build:appTwo prod-side issues this merge does NOT fix
bot/package-lock.jsonis out of sync with its ownpackage.json(chart.jsmissing) —npm cicannot succeed on main. Needs a small PR straight to main.V1.1.1,V1.1.3,V1.1.4while having appliedV1.1.5–V1.1.7. Before any promotion, confirm those three are safe to apply after the higher numbers.Does not change production.
Closes: bd-43478