Give every profile a URL, and close the ?about= injection - #63
Merged
Conversation
The directory card was written before `docs/spec/profile-and-credentials.md` existed and rendered a model that no longer does: a profile-level `verified`, a `certified` flag that was declared and never read, and a `role` field the schema calls `headline`. Same failure `docs/scope.md` had before #54 — written against a superseded vocabulary, and reading as current because it once was. Three variants were prototyped on a throwaway route and the roster won, so this commits the roster and deletes the rest. Rows rather than a grid of cards, because the job here is comparing practitioners and a grid is a poor shape for it: each card is read on its own and nothing lines up between them. In rows the credentials sit in a column, so "who has actually been checked" is a vertical scan rather than eight separate readings. Two changes are load-bearing rather than stylistic. The badge now sits on the credential column instead of beside the person's name — the spec calls that a UI constraint the directory work inherits, because a narrow badge placed where it reads as a whole-profile endorsement is misleading no matter what the clearing rule does, and no schema rule can fix that reading. And the badge is derived rather than read: there is no profile-level `verified` column, so `hasVerifiedBadge` computes it from the credential rows, as does the "Verified only" filter. The types now mirror the columns `anon` is granted and nothing else, so a component cannot reach for a field the directory will never be able to select. That adds `countryCode` and per-credential `verified`, `earnedAt` and `evidenceUrl`, and removes `certified` — which the spec derives rather than stores. The location filter groups on `countryCode`, not on `location`. `location` is deliberately free text at whatever granularity the practitioner chose, so "Sydney" beside "Bengaluru, Karnataka, India (remote)" will never collapse into a usable set of chips; `country_code` is the column that exists for this. Filters are now grouped and labelled, because a country, a focus area and "Verified only" are three different kinds of claim and one flat row of chips read as an undifferentiated pile once countries were added. Two states the old card could not draw now render: a profile with no credentials at all, which nothing in the DDL forbids, and one that is entirely working-towards, which must not read as a failed check because that group is one the directory exists to include. The Enquire button carries `?about=` to `/contact`, which names the practitioner in the page, the mail subject and the body. Enquiries still route through Bluehex — no address is published on a profile. This makes `/contact` server-rendered per request rather than prerendered; the alternative, reading the param client-side under Suspense, keeps the static shell but flashes a fallback over the page's main content. Verified by temporarily populating the directory with four profiles covering the awkward cases and re-running the suite, since an empty directory exercises no rows at all: axe clean on the row markup in both viewports, the badge on exactly the profile whose earned credentials were all verified and absent on the mixed one. No flag is drawn yet — `countryCode` powers the filter, and the SVG assets are their own item in scope.md. `Practitioner` keeps its name despite CONTEXT.md noting it conflates the human with the record; that rename reaches the component, its file and its props, and belongs with #53.
The parameter was echoed straight into the page, the mailto subject and the mail body, so `/contact?about=<anything>` rendered attacker-supplied text as Bluehex's own copy. It now carries a profile id and is resolved to a name against `practitioners`; anything that does not match renders no banner at all. Carrying the id rather than the display name is right for a second reason that outlives this fix. Names are not identifiers — two practitioners can share one and the enquiry would not say which — which is the same argument that keeps the display name out of a profile URL. Nothing legitimately linked here, because the directory ships empty, so there is no compatibility concern.
`profilePath` generates `/p/mara-ellison-9f3c1a`, where the trailing short id is what resolves and the slug is decoration — a rename changes the URL without breaking the old one, and the route serves a canonical redirect when the slug no longer matches. The id is the first six characters of the row's uuid and must never be derived from the name, since hashing the name would move the id whenever the name changed, which is the exact failure the scheme exists to prevent. What carried the decision was analytics rather than depth. A full page adds only three fields over the roster row, so on substance alone it does not earn a route; but a path segment gives referrer data, which is how you learn an employer arrived from a candidate's job application, and indexability, which is how somebody searching for a Claude consultant lands on a profile at all. Expand-in-place can never answer either. The row's control changes from Enquire to View profile. One call to action per surface: the directory gets you to a profile, the profile gets you to enquire. `/p/[handle]` here is prod-guarded and fixture-backed — a drawing for the real implementation to replace, not the implementation. This leaves a deliberate dangling link, and it is the thing to watch: nothing breaks today because the directory ships empty, so no row and therefore no link renders, but the profile route has to land before the first practitioner is added or the only control on a row 404s and enquiries have no path at all.
This was referenced Aug 15, 2026
davidtaing
commented
Aug 15, 2026
davidtaing
left a comment
davidtaing
left a comment
Collaborator
Author
There was a problem hiding this comment.
Automated review — pass run by Codex (via the codex-rescue subagent), verified and posted by Claude Code (Opus 5). Findings were checked against the source before posting.
The ?about= fix holds. Resolving the id against practitioners closes the reflection; an array-valued ?about= falls through the typeof check; and URLSearchParams already percent-encodes &, CR and LF, so nothing reaching the mailto: can introduce a header. That commit is the strongest part of the PR.
The blocker is elsewhere: this branch does not compile. handles.ts imports a fixture that arrives in #65, and #63 is the bottom of the stack, so main breaks on merge and stays broken until #65 lands. pnpm test:e2e has never run here and cannot until that is fixed.
1 major, 8 minor, 2 nits.
Next: decide whether the /p/ tree belongs in #63 at all — see the comment on handles.ts:21. Everything else is independent of that call.
| .replace(/[^a-z0-9]+/g, "-") | ||
| .replace(/^-|-$/g, ""); | ||
|
|
||
| return `/p/${slug}-${person.id.slice(0, 6)}`; |
Collaborator
Author
There was a problem hiding this comment.
minor — The short id is six hex characters with nothing guaranteeing it is unique, and the resolver (findByHandle) returns the first row whose uuid starts with those six. A collision therefore does not 404 — it silently serves someone else's profile, badge and all, at a URL the other practitioner pasted into a job application. Given the badge is what the directory sells, that is the wrong failure mode to leave to chance.
Six hex characters is a 16.7M space, so by the birthday bound the collision probability is roughly n²/(2·16.7M): about 0.03% at 100 profiles, 3% at 1,000, 50% at ~4,800. Small now, and the directory ships empty. But the URL format is the expensive half to change later — every published profile link is a permanent artifact of whatever is decided here, which is the same argument the docstring above makes for not deriving the id from the name.
Cheapest resolutions, in order of how little they cost today: widen the slice (8 characters puts the 1,000-profile case at 0.001%); or make the prefix a real column with a unique constraint and let the insert retry on conflict, which turns a silent wrong-profile into a loud write error at the only moment anyone can still do something about it. Worth naming in docs/spec/profile-and-credentials.md either way, since #53 is the migration that has to honour it.
Collaborator
Author
There was a problem hiding this comment.
Agreed, and left open deliberately — the fix is a schema decision and there is no schema in this PR. Nothing has been bolted on to close it.
Why not just widen the slice to 8. It would move the number without changing the kind of answer: still probabilistic, on the one property the directory cannot be probabilistic about. findByHandle returns the first row whose uuid starts with the prefix, so a collision does not 404 — it serves a different practitioner's profile, and it serves the verified badge along with it, at a URL the wronged practitioner pasted into a job application. AGENTS.md puts that badge at the centre of the product. A unique column with retry-on-conflict at insert is the qualitatively different fix, because it converts a silent wrong read into a loud write error at the only moment anyone can still act on it, and that lives in #53's migration.
There is a second reason this needs your call rather than mine. docs/spec/profile-and-credentials.md has a section headed No slugs, and no profile identity problem, decided by inspection on the grounds that "there is no profile URL, so there is nothing for a slug to name". Its gate reads:
Gate: the first per-profile route. A profile page needs a stable identifier that is not the display name, and that decision should be made when the page is, not before.
This PR is that gate opening, and it opened it without the spec noticing. So the identifier is not only unenforced, it is undecided in the binding document — which makes editing that section a decision rather than a step, and not one to take inside a review-resolution pass. It wants a short spec section fixing the identifier scheme (width, uniqueness, and what happens on conflict), and #53 then has something to honour.
What did land, in 0be10d8, is that the profilePath docstring stops implying safety it does not have — it now says outright that nothing guarantees the six characters are unique, that findByHandle returns the first match, and that the enforcement is open on this review. That is a truthful comment, not a mitigation.
Happy to write the spec section and open the ticket if you want it in this pass; leaving it to you since the width and the conflict behaviour are yours to set.
`/p/[handle]` imported `launchPopulation` from `src/app/prototype/directory/fixtures`, which does not exist on this branch — it arrives two pull requests later. The branch did not build, and this is the one that merges to `main` first, so `main` would not have built either. `pnpm lint` passes on it, which is why three sessions missed it: lint is not a typecheck. The import is the defect rather than the missing module. A production route reading a throwaway fixture is what AGENTS.md warns about, and the machinery around it was all there to keep that premise standing: a `NODE_ENV` guard, a `noindex`, a panel explaining that a real build would redirect, and a breadcrumb pointing back into `/prototype/`. Removing the premise removes the requirement, so those go with it rather than being carried. The lookup now reads `practitioners`, which is empty and stays empty until real people are in it. Every handle 404s today. That is the same emptiness the directory renders its invitation card for. Two things the removed panel was standing in for are now real. A non-canonical slug redirects to the canonical path instead of rendering both with an explanation, which is what `profilePath`'s own docstring already claimed happened. The share button's origin moves out of a hardcoded string and into `site.ts`, where that file's comment said it belonged once the route was real. `pnpm build` and `pnpm lint` both pass on this branch standing alone, for the first time.
davidtaing
added a commit
that referenced
this pull request
Aug 15, 2026
The lede invited the visitor to click *View profile* and reach the person's page. Since `4ef5ef4` on #63, `/p/[handle]` resolves against the real practitioners rather than this prototype's fixtures, and that array is empty — so the click 404s and the copy was promising a page that does not exist. The whole point of accepting the dead link rather than building a prototype detail route was that the seam should be legible. Copy that hides it costs more than the note in NOTES.md buys, so the lede now says the row points at the person's real URL and that clicking it 404s because there are no real practitioners yet. Still one sentence longer than it was, not an explainer.
`earnedLabel`, `CredentialMark` and the tick path were defined twice by this branch — once in `practitioner-directory.tsx` for the roster row and once in `src/app/p/_lib/credential-mark.tsx` for the profile page. They had already disagreed inside the commit that introduced them: `month: "short"` against `month: "long"`, so the same credential read "Earned Aug 2026" on the roster and "Earned August 2026" on its own page. The date format is cosmetic; the mark is not. It carries the three-state distinction between working towards, earned but unchecked, and verified, including the screen-reader text that states it — which per AGENTS.md is the product. Two files stating that with nothing keeping them in agreement is the defect, and the drift was the demonstration. The copy existed because production code cannot import from a route's `_lib`, which is the right instinct about the direction of the dependency. Hoisting it to `src/components/credential-mark.tsx` fixes the direction and the duplication together, and both surfaces now import it. The shared `earnedLabel` keeps the short month, because the roster is the surface with the narrower column and it reads correctly on the page too.
Two things the roster got wrong about words the rest of the page defines. The badge said "Bluehex checked these" on a page whose own paragraph defines **Verified** as "Bluehex has checked that credential against the certificate that issued it", whose filter chip reads "Verified only", and whose profile page says "Verified by Bluehex". A visitor filtering by "Verified only" got back rows on which nothing was labelled verified and had to infer the two phrases were one claim. AGENTS.md treats the wording as the value proposition rather than as copy, so the roster was the surface that diverged from the standard, not the standard. "Verified by Bluehex" keeps the badge attached to the credentials it sits with, which is the load-bearing part. The search index had no country in it, so the search box and the Location filter disagreed about the same fact: typing "Australia" matched nobody while the chip built from that person's `countryCode` selected them. Worse than useless, because `matchesQuery` requires every term to hit — adding a country word to an otherwise good query made results vanish rather than narrow. The country name goes into the haystack; the raw code stays out, since two-letter queries would hit more than they were aimed at. The note claiming `/p/` does not exist goes with them. It does exist, as of the previous commit, and so does the link's destination.
Two independent defects in the pair of functions this branch added.
`profilePath` returned `/p/-9f3c1a` for any name left with no ASCII residue after the diacritic strip — "李雷" and "Ω" both slug to the empty string, where "José Ávila" survives as "jose-avila". A leading bare hyphen where the readable half should be undercuts the whole reason the docstring gives for having a slug, which is that the URL is readable enough to paste into a job application. Not hypothetical for this directory in particular: the location filter added on this same branch exists because practitioners are expected outside Australia. The slug and its separator are now dropped together, giving `/p/9f3c1a`, which `findByHandle` resolves unchanged.
`countryName` promised to fall back to the code for an unknown region, but `Intl.DisplayNames` fails silently rather than throwing on the input most likely to arrive. Confirmed on Node 24.19: `of("AU")` gives "Australia", `of("au")` gives back `"au"`, and only a structurally invalid code such as `of("usa")` throws. So the `catch` never covered a lowercase code, and one would have rendered a filter chip reading `au`. Uppercasing before the lookup closes it; a check constraint on the column belongs with #53, and the docstring now says so.
`URLSearchParams.toString()` serialises as `application/x-www-form-urlencoded`, which writes a space as `+`. A mailto query is not form encoded: RFC 6068 specifies percent-encoding, in which `+` is a literal plus. Clients split on it — the ones that follow the RFC open a compose window reading "Enquiry+about+Mara+Ellison", the ones that reuse form decoding show spaces. The line is pre-existing, but this branch rewrites it and adds a third value that passes through it, so it both widens the exposure and makes the mangling more visible. A practitioner's name is the last thing that should reach Bluehex's inbox looking like that. Replacing every remaining `+` with `%20` is safe because a plus a visitor actually typed is already `%2B` by that point — confirmed by round-tripping a body containing "C++ work", which survives. `URLSearchParams` still builds the query, so it still percent-encodes `&`, `?`, CR and LF and no field value can introduce a second mailto header.
The button set the label unconditionally. `navigator.clipboard` is undefined on any non-secure origin — a dev server reached over a LAN address rather than localhost is the everyday case — and the optional chaining then short-circuited straight into `setCopied(true)`; where the object does exist, `writeText` returns a promise that rejects on a denied permission or an unfocused document, and the `void` discarded the rejection along with it. Both paths reported a copy that had not happened. Awaiting the write puts the confirmation behind the fact, and a failure leaves the label alone. This was ranked a nit against a file that described itself as throwaway, and it is worth more than that now the route is real: pasting a profile link into an application is the reason the route exists, so a button that lies about having copied breaks precisely the flow it is there for. Three comments still described the prototype that is gone. Two pointed at a `NOTES.md` that does not exist on this branch, and one said the enquiry banner would never appear because "these are fixture people" — the page resolves against the roster now, so it will. They say what they mean instead of deferring to a file that is not here.
Collaborator
Author
Review resolved — 10 of 11 threads closed, 1 left openTen fixed and resolved, one left open because the fix is a schema decision and there is no schema in this PR. Line numbers had moved: 4ef5ef4 landed between the review and this pass, so every finding was re-anchored against the current head before being answered. Already addressed by 4ef5ef4, verified rather than taken on trust
Fixed in this pass
Left openThe six-hex short id has no uniqueness guarantee (thread). Real, and not patched around. Widening to eight characters moves the number without changing the kind of answer, and the failure mode is serving the wrong practitioner's profile with its verified badge — the one thing the directory cannot be probabilistic about. The deterministic fix is a unique column with retry-on-conflict, which is #53's migration. It is also undecided rather than merely unenforced: Not re-openedThe header-injection question on the mailto construction. The review checked it and recorded the negative result: input value sanitisation strips CR/LF before the handler runs, and Still a draft, and left that way. |
davidtaing
marked this pull request as ready for review
August 15, 2026 08:57
davidtaing
added a commit
that referenced
this pull request
Aug 15, 2026
The lede invited the visitor to click *View profile* and reach the person's page. Since `4ef5ef4` on #63, `/p/[handle]` resolves against the real practitioners rather than this prototype's fixtures, and that array is empty — so the click 404s and the copy was promising a page that does not exist. The whole point of accepting the dead link rather than building a prototype detail route was that the seam should be legible. Copy that hides it costs more than the note in NOTES.md buys, so the lede now says the row points at the person's real URL and that clicking it 404s because there are no real practitioners yet. Still one sentence longer than it was, not an explainer.
davidtaing
added a commit
that referenced
this pull request
Aug 15, 2026
Second of a four-PR stack. Stacked on #63 — review that first; this PR's own diff is the `src/app/prototype/profile/` directory and nothing else. Three shapes for the surface a practitioner fills in, drawn and narrowed to one. `pnpm dev`, then `/prototype/profile`. **This is a drawing, not an implementation.** Prod-guarded, so it cannot ship. No auth, no database, no write path — the real editor needs application auth first. Kept so that work has something to build from, and to be deleted as it is replaced. The verdict, the two shapes it beat and what the built version has to be held to are in `NOTES.md` beside it. Self-contained: nothing here imports from `@/lib` or `@/components`, so it does not actually depend on #63 — it is stacked only to keep the review order linear. `pnpm lint` passes. `pnpm build` and `pnpm test:e2e` have not been run.
davidtaing
added a commit
that referenced
this pull request
Aug 15, 2026
Third of a four-PR stack. Stacked on #64; this PR's own diff is the `src/app/prototype/directory/` directory and nothing else. `/prototype/directory` renders the **real** `PractitionerDirectory` against three invented profiles rather than a copy of it, so it answers whether the shipped design survives being nearly empty. Prod-guarded. This is the one prototype that genuinely depends on #63, since it imports the production component and the reshaped `Practitioner` type. The population is realistic in the one way that matters: **nobody has finished a Claude Certification.** Two people have started the courses and none have completed one, so every earned credential here is an Anthropic Academy certificate and every Certification is in progress. ### The finding, which is about the model rather than the drawing `scope.md` poses the badge as a fork — Certification-only and empty at launch, or vetted-and-Claude-capable and a different product. **The first branch is not reachable without a schema change, and the mechanism already picked the second.** The rollup counts any earned credential, and the spec is explicit that the two sources differ in weight rather than in kind, so badges appear from week one earned entirely on Academy certificates. Counted against this population: **three profiles shown, two carrying the badge, none holding an earned Claude Certification.** So the open question is not "will anyone have a badge". It is whether Bluehex is willing to say the badge means what it already does — that a human checked the credentials on this profile, whatever they are. Making it Certification-only would need the rollup narrowed to `source = 'Claude Certification'`, which contradicts a settled spec and would deny the badge to everybody at launch. If that is wanted it has to be argued as a change to the model, not fixed in the lede. ### Also in NOTES.md The identifier scheme, whether a name in the URL increases scraping exposure (it does not; what it does affect is withdrawal, which belongs to #52), and a full post-mortem of a Seek-style overlay that was built with parallel and intercepting routes and **did not work**. Three real causes are written down. Do not re-attempt it inside this app without first reproducing against a bare Next app. The overlay being cut has a consequence worth pricing before the profile page is built for real: search and filter state is local React state, so going to a profile and back loses whatever the visitor typed and ticked. An overlay would have preserved it for free. Lifting search and filters into the URL should land *with* the profile page, not after it. `pnpm lint` passes. `pnpm build` and `pnpm test:e2e` have not been run.
davidtaing
added a commit
that referenced
this pull request
Aug 15, 2026
The deploy workflow fires on a push to `main` and nothing else. That is the right default and stays the default — shipping should normally be a consequence of landing on `main` rather than a button someone remembers to press. It leaves one gap, and we hit it today. A workflow disabled by hand creates **no run** for a push that arrives while it is off — not a queued run, not a skipped one, nothing at all. So when `Vercel Deploy` was re-enabled after #63–#66 merged as a stack, there was no run to re-run and `main` sat ahead of production with no way to reconcile the two except pushing a commit whose only purpose was to be pushed. Re-running the most recent deploy run is not a substitute and is worse than doing nothing: it redeploys the commit that run was for, which here is `497cde4` from 13 August. ## The ref guard `workflow_dispatch` runs against whichever ref the caller picks in the dropdown, and every step in this job passes `--prod`. Without a guard, selecting any branch there ships that branch to production. `if: github.ref == 'refs/heads/main'` sits on the job rather than inside the deploy step, so a wrong ref costs a skipped run rather than a half-finished one. ## Verification Parsed with `yaml.safe_load`: triggers are `push` and `workflow_dispatch`, and the job carries the ref condition. The workflow itself is unchanged below that — same pinned action SHAs, same concurrency group, same steps. Note that merging this will itself trigger a production deploy of `main`, since it is a push to `main` and the workflow is enabled again. That is the intended outcome as well as the fix.
This was referenced Aug 20, 2026
davidtaing
added a commit
that referenced
this pull request
Aug 22, 2026
Closes #53. The first query this repository makes. The directory stops rendering the empty array in `src/lib/practitioners.ts` and reads Postgres through the anonymous client. **Both surfaces, in one PR.** #53 scopes itself to the directory, and the profile page came with it because `findByHandle` read the same module: shipping the directory alone would have listed five seeded people whose every *View profile* link 404s. The prototype's own NOTES call them "one surface, because they stopped being separable". **The prototype is kept.** #53's Done-when says to delete `src/app/prototype/directory/` and `src/app/prototype/catalogue.ts`; that was overridden. Both still compile and still render — the prototype is now the only place the *Not earned* control has a catalogue full enough to reveal, since production's is empty until a stack is seeded. `catalogue.ts` translates its invented `source` values into the `kind` / `platform` pair rather than restating twenty-four invented rows in the new shape. ## The query Named columns throughout, verified against the migrations rather than the issue: | table | columns read | | --- | --- | | `practitioners` | `id, name, headline, location, country_code, bio, focus, availability, website_url, github_url, linkedin_url, booking_url` | | `practitioner_credentials` | `id, catalogue_id, earned_at, verified, evidence_url_public` | | `credential_catalogue` | `id, kind, platform, label, course_url, active, sort_order` | | `practitioner_services` | `id, catalogue_id, label` | | `service_catalogue` | `id, label, active, sort_order` | **Nothing filters on `status`.** It is not in the `anon` grant, so `.eq("status", "approved")` is a `42501` rather than an empty result. `practitioners_read_approved` is the filter and the children follow their parent through `profile_is_approved()`, which is why the listing has no `where` clause at all — it looks wrong and is exactly right. `availability` and the four link columns are read and carried into the model and are still **not rendered**, except `booking_url`, which the profile page already drew before this change. They were left rather than missed; that is #84 and #85. ## Rendering and caching **The directory** is prerendered and revalidated daily (`export const revalidate = 86400`; the literal is repeated in each route because the value must be statically analysable). **`/p/[handle]` returns an empty `generateStaticParams` with `dynamicParams` at its default.** Nothing is prerendered; a profile page is rendered on its first request and cached from then on, so a newly approved profile appears with no rebuild and no deploy. The fact that settles it, because it is easy to get backwards: **`generateStaticParams` is not called again during revalidation**, so enumerating handles there could never have been the mechanism by which new profiles arrive — `dynamicParams` is. `dynamicParams = false` would 404 every profile there will ever be. Returning `[]` is also what the API reference requires in order to revalidate paths at runtime at all, so this is the sanctioned ISR shape rather than a workaround. **No `connection()`.** #53 says this read is that document's worked example, and the shape does match — an anonymous public query touching neither cookies nor headers. But what the API is *for* is a component that must produce different output per request, and these must not: they are cached for a day deliberately, and `await connection()` would opt both routes out of prerendering and undo the rendering decision rather than implement it. `cacheComponents` is left off, per #117. **Three concerns, three mechanisms, and only two of them exist yet:** 1. **A new profile appears** — on-demand render at first request via `dynamicParams`. Working. 2. **An ordinary edit** — the daily clock. Working, and a staleness preference: seeing your own edit tomorrow is poor and acceptable. 3. **Revocation** — `status` leaving `approved`, or a credential's `verified` going false. **Nothing purges these pages today.** The clock *bounds* that window at 24 hours; it does not close it, and a pulled badge sitting in a cache for a day is the silent, fails-open failure #53 is explicit about. This PR does not fix that and should not be read as fixing it. The tag names are recorded as a contract where the caching is configured — `practitioners` for the listing, `practitioner:<id>` for a profile page — with a note that a status or `verified` change purges both and an ordinary edit purges neither. No tagging plumbing was built: attaching a tag needs either a `fetch` of ours to hang `next.tags` on (the queries go through `supabase-js`, so there is none) or `cacheTag()` under `cacheComponents`, which is an application-wide decision now owned by **#117**. The writes that will call the purge are **#14**. One thing worth writing down before somebody writes that call: it wants **`updateTag`**, which expires the entry immediately — **not** `revalidateTag(tag, "max")`, which serves the stale response one more time. For a pulled badge that is the pulled badge going out once more, which is the entire failure being closed. Bare `revalidateTag(tag)` with no second argument is deprecated. **CI keeps no database, and the build degrades.** Both reads answer empty when Supabase is unconfigured — no environment variables at all — which preserves the AGENTS.md property that `next build` passes with none set. A **configured** deployment whose query fails still throws, and that asymmetry is deliberate: swallowing the error would render an empty directory indistinguishable from a true one, while failing the render fails the build and leaves the previous deployment serving. ## Reconciling `src/lib/practitioners.ts` It becomes the public view model and holds no rows. `practitioners` and `credentialCatalogue` are gone; `src/lib/directory.ts` is the read and `src/lib/directory-mapping.ts` is the pure half of it, split so the ordering and null rules are assertable in `pnpm test` rather than only against a stack. - **`Practitioner` → `Profile`.** `CONTEXT.md` already said the old name conflated the human with the record; the type describes a row, and the row is the profile. Nine files, mechanical. - **`CatalogueEntry.source` → `kind` + `platform` + `courseUrl`**, catching the type up with #103. `credentialSource(entry)` prints the one line both public surfaces used to take from `source` — the weight for a certification, the platform for a course — so the picker's `<optgroup>` labels and the profile page cannot describe an entry two different ways. - **`isCertified` joins `hasVerifiedBadge`.** Both derived, neither stored, neither with a column to disagree with. Nothing renders `certified` yet; it exists so the derivation has one home. - **`Profile.services` is `string[]`**, because `practitioner_services` carries both catalogue rows and custom labels. The closed `services` array survives as the editor's vocabulary and as the list the table was seeded from. - **REAL PEOPLE ONLY survives**, restated in the module header and in AGENTS.md: the rule moved to where the rows now are. No invented person is in this diff. `supabase/seed.sql` remains the one place invented people are allowed, because it never runs against hosted. Filter chips now come from `service_catalogue`: built from services somebody actually offers, ordered by `sort_order` rather than `sort()`. Two consequences fall out of the chips being the table rather than the labels on the rows — a **custom** service renders and can never become a chip, which is promotion working rather than a filter somebody forgot, and a **retired** entry stops being a chip while every profile still carrying it keeps rendering the label. ## How the silent failures were proved Against the eight seeded profiles from #110, on the local stack. **An unapproved profile does not appear.** `anon` sees five rows — Devon, Hollis, Mara, Priya, Toby. Ines (`pending`), Rafael (`rejected`) and Sabine (`withdrawn`) are absent from the listing, from the prerendered HTML and from the flight payload, and `/contact?about=<Sabine's id>` renders no banner. The two refusals were checked directly rather than assumed: `select=*` and `status=eq.approved` both answer `42501 permission denied for table practitioners`. **`evidence_url` never reaches the browser.** Checked in the built artifact rather than in the rendering, script payload included: `evidence_url` appears 0 times, and `certificates/mara-ellison-mcp` — the credential whose `evidence_public` is false — appears 0 times. The three rows where the practitioner opted in do appear. The mapper is asserted to produce exactly `{entry, earnedAt, verified, evidenceUrl}`, so a future spread cannot carry the column in either. **The badge derives correctly across the population.** Two profile-level pills in the markup, on Mara and Priya. Toby is held back by one unverified credential while still showing a verified *credential* mark — which is the pair that proves the rollup is "every credential verified" rather than "any". Devon holds nothing and carries no badge. The pills sit in the credentials column, after the `</h3>` that closes the name block, never beside the name. **Zero rows renders what it renders today.** The no-Supabase build produces the invitation card, the "first profiles are being verified now" line and the "Certified, or working towards it" copy, with no rows, no `<h3>` and no filter chips. There is now an e2e test asserting exactly that. ## Verification All five run separately, real exit codes, no pipes. | command | exit | | --- | --- | | `pnpm lint` | 0 | | `pnpm test` | 0 (231 tests, 10 files) | | `pnpm build` | 0 | | `pnpm build` with both `NEXT_PUBLIC_SUPABASE_*` unset | 0 | | `pnpm test:db` | 0 (185 tests, 11 files) | | `pnpm test:e2e` | 0 (20 tests) | New unit tests cover the badge rollup against each seeded shape, the `certified` derivation, catalogue ordering, the row→view-model mapper, credential and service ordering, and chip ordering. `directory-mapping.test.ts` also carries a compile-time assertion that the hand-written row types still match `database.types.ts`, so a column changing shape fails the build rather than the query. **The select string is a literal on purpose.** `supabase-js` parses it at the type level to derive the row shape, and can only do that from a string literal type — assembled with `+` the parts widen to `string`, the parser gives up, and `data` arrives as `GenericStringError[] | null`, which is what a cast would then have to paper over. `as const` on each fragment keeps the check on: a column that does not exist, an embed that is not a relationship, or a name that changed under a migration are all build failures naming the column and the table. **`pnpm test:e2e` was red before this branch, and was fixed deliberately.** `e2e/routes.spec.ts` asserted the home page's `h1` is "We only do Claude."; it has read "Claude Specialists." since long before #53 — "We only do Claude." is the tagline underneath it. Nothing caught the drift because the `End-to-end tests` workflow is disabled by hand. The assertion is corrected with a comment saying so, and one test is added for the empty directory, which is the regression #53's Done-when actually asks for. ## Finding: the seeded profiles all share one handle `profilePath` takes the first six characters of the uuid, and `supabase/seed.sql` keys its eight profiles `22222222-0000-4000-8000-00000000000N` so a stray uuid in a log says which table it came from. All eight therefore have the handle `222222`, so **every *View profile* link on a freshly reset local directory resolves to the same person** — Mara, since the ids are now ordered. `/p/toby-nakamura-222222` 307s to `/p/mara-ellison-222222`. This is #63's unenforced uniqueness made concrete rather than a new bug: the resolver has always returned the first row matching those six characters. Real uuids are random and collide far more rarely, but "far more rarely" is what #63 exists to replace with "never". I did not change the seed — the ids are pinned by `tests/db/practitioner-seed.test.ts` and belong to #110 — so the mitigation here is only that `listProfileIds` orders its rows, making the wrong answer the *same* wrong answer on every regeneration rather than one that changes with the query plan. A cached page that served a different person each time would be harder to recognise as the collision it is. ## Documentation The settled findings in `src/app/prototype/directory/NOTES.md` are migrated into `docs/spec/profile-and-credentials.md` as **The public surfaces: the roster, the profile page, and how they read**, beside the `## The editor` section #71 wrote, covering the roster shape and the five variants that lost to it, the chip vocabulary, why `/p/<handle>` is justified by having a URL rather than by depth, the identifier and its collision, the rendering and caching decisions above, and what the surfaces still owe. NOTES.md keeps the working record — the variants, the overlay that was built and cut, the debugging — under a header saying the spec wins where they disagree. `AGENTS.md` and `CONTEXT.md` are updated for the type rename, the derivations, and the fact that a query now exists.
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
1 participant
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.
Base of a four-PR stack. This is the production half; the three prototype surfaces stack on top of it.
Review the middle commit on its own if you review nothing else.
?about=was echoed straight into the contact page, the mailto subject and the mail body, so/contact?about=<anything>rendered attacker-supplied text as Bluehex's own copy. It now carries a profile id and is resolved to a name againstpractitioners; anything that does not match renders no banner. Nothing legitimately linked there, because the directory ships empty, so there is no compatibility concern.The other two commits reshape the public view of a profile to mirror the columns
anonis granted indocs/spec/profile-and-credentials.md— nothing more — and give a profile its own URL.profilePathgenerates/p/mara-ellison-9f3c1a, where the trailing short id resolves and the slug is decoration, so a rename changes the URL without breaking the old one. The id is the first six characters of the row's uuid and must never be derived from the name: hashing the name would move the id whenever the name changed, which is the exact failure the scheme exists to prevent.What carried the decision to give profiles a URL was analytics rather than depth. A full page adds only three fields over the roster row, so on substance it does not earn a route; a path segment gives referrer data, which is how you learn an employer arrived from a candidate's job application, and indexability, which is how someone searching for a Claude consultant lands on a profile at all. Expand-in-place can never answer either.
The one thing to watch
/p/is prod-guarded and fixture-backed — a drawing, not the implementation. The roster's control changed from Enquire to View profile, which leaves a deliberate dangling link. Nothing breaks today because the directory ships empty, so no row and therefore no link renders. But the real profile route has to land before the first practitioner is added, or the only control on a row 404s and enquiries have no path at all. There is aNOTE:at the link saying exactly this.Not verified
pnpm lintpasses.pnpm buildandpnpm test:e2ehave not been run against this branch — the e2e suite exercises an empty directory, so no roster row is covered by it either way.