Chase fournier/grade migration - #10
Open
Chase-Fournier wants to merge 20 commits into
Open
Conversation
Phase 4 from CHANGES_.md. The important addition is `instructorSlug` on /v0/grades and /v0/grades/summary. Filtering by `instructor` compares a name string, and the same professor is spelled four different ways across the registrar exports, Testudo, and PlanetTerp -- "Walsh, Shane Bolles" against "Shane Walsh". A professor page built on that returns zero rows for a large share of instructors, and every failure is silent: an empty section, not an error. Slug and id resolve through instructor identity instead, so they cannot miss. The name parameter stays and is documented as unreliable. Two new groupings, `instructorOverall` and `instructorTerm`, back the professor page's headline GPA and its trend chart. Neither was expressible before -- nothing in the database produced a per-instructor rollup across courses at all. Filters the chosen grouping cannot honor now return 400 instead of being dropped. Asking for one professor's CMSC132 numbers and silently receiving their average across everything they have ever taught is not something the caller can detect. `nameSearch` does substring matching over the normalized name column, backed by the trigram index. This is what lets the site stop downloading every active instructor into browser memory on each page load. Two behaviour changes worth calling out in review: * `minStudents` now filters on `graded` rather than `total`. Before Fall 2017 the registrar's total counts students whose outcome was never categorized, so it is not comparable across eras; `graded` is the letter-grade count and is also the GPA denominator, which makes the threshold mean the same sample the GPA came from. * Cache capacity 124 -> 4096, split into two caches. 124 was sized for department prefixes and a few course codes; per-professor keys would have thrashed it to a near-zero hit rate with every request reaching Supabase. Course search gets its own cache so professor-page traffic cannot evict it. docs.md is updated. docs.html is NOT -- the two are maintained by hand in parallel and have already drifted; see CHANGES_.md 7.8 on generating one from the other. Builds and vets clean. Not exercised against a live database. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
docs.md and docs.html were maintained by hand, in parallel. That worked while the API had seven stable endpoints; the grade work added 235 lines to one and 439 to the other in a single commit, and they had already drifted before the review endpoints made it worse. Markdown is the source now. docs.html stays committed so deploying the binary needs no generation step, and `go generate ./...` rebuilds it. Two things are reproduced deliberately rather than improved. The heading anchor scheme is the odd one the previous generator produced -- lowercase, every run of non-alphanumerics to a single hyphen, leading and trailing hyphens left on, so a heading of `/v0/courses` becomes "#-v0-courses-". Trimming would be tidier and would break every internal jump link and any external link into a section. The page shell is byte-identical apart from a title and a viewport meta. Raw HTML passes through, because several table cells embed <ul><li> lists that markdown cannot express inside a cell. Escaping them rendered the markup as literal text. The generator fails the build if an internal link points at a heading that does not exist. docs.md is one large table of contents pointing into itself, and a renamed heading otherwise breaks those links silently. Dropped: the highlight.js `hljs-*` spans the old output wrapped around every code block. docs.css styles none of them, so they were a few hundred lines of markup with no rendered effect. Verified against the previous docs.html: every heading id from the old file is present, and the only vocabulary differences are three phrases I had already rewritten in docs.md. Duplicate ids in the old file -- it had seven headings called "Query parameters" all with id="query-parameters" -- are now suffixed, which also makes the document valid HTML. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 10 from CHANGES_.md, and the parts of 13/14 that live in this service. The API was a stateless, unauthenticated, read-only proxy. This adds everything a write path needs, all of it on /v1 so that /v0 stays the permissive cacheable surface it has always been. CORS is now per group: an allowlist on /v1, unchanged on /v0. A permissive policy on a write endpoint means any page on the internet can make a visitor's browser submit a review. The service-role key lives in exactly one type, WriteClient, and nothing outside that file sees it. Putting the write path here rather than in Edge Functions was a deliberate trade -- one API, one deploy, one language -- but it converts key containment from a property into work, and that file is where the work is concentrated. RLS is the backstop: 0008 is written assuming a handler here is one day wrong. Notable decisions: * The submit response is identical whether or not that address already reviewed the professor. Anything distinguishable turns the endpoint into an oracle for "did person X review professor Y", which is the privacy property the hashing exists to provide. * Email is queued in email_outbox and sent from there, never inline. Two reasons: a submit that 500s because a mail provider was slow loses the review, and -- per your note about the Brevo cap -- hitting the daily cap now defers a send rather than dropping it. I did not make the cap auto-approve submissions: that would make exhausting the cap a way to bypass verification, and verification is what backs the UMD-affiliation claim, the per-email dedupe, and most of the abuse defences. The knob exists as REVIEW_ALLOW_UNVERIFIED_ON_EMAIL_CAP and defaults off, with a loud boot warning when it is on. * Rate limiting is a Postgres counter table. Cloud Run autoscales, so an in-memory limiter is per-instance and bypassed by retrying until you hit a cold one. There is a per-instructor limit as well as per-IP and per-email; the per-person limits do nothing against thirty people arriving at once to bury one professor. * A deterministic pre-filter runs before any model sees a review. Links, emails and phone numbers are refused outright with no model call. Suspected prompt injection and anything alleging misconduct about a named person escalate to a human regardless of what a classifier concludes -- a rule in code, not an instruction in a prompt, because prompt instructions are what an injection attacks. * Thresholds: auto-approve needs confidence >= 0.90 AND zero policy categories AND zero pre-filter flags; auto-reject needs >= 0.85. Asymmetric because a wrong rejection annoys one student who can appeal, and a wrong approval publishes something defamatory about someone who never opted in. Both gates default OFF -- shadow mode records the classifier's opinion with applied=false and a human decides, which is what makes enabling automation a config change rather than a code change. A test asserts that ordinary reviews pass the pre-filter untouched, so "the vast majority should not be flagged" is checked rather than hoped for. * Boot-time config validation refuses to start when REVIEW_TRIAGE_TIMEOUT_SEC <= REVIEW_TRIAGE_RETRY_MAX_SEC. Backwards, the sweeper escalates every quota-blocked review before its retry fires and the retry queue is dead code that looks like it works. The "TODO: Add logger, auth with keys" in main.go is now done. /v1 request logging hashes the client IP rather than recording it. docs.md gains a /v1 section; docs.html regenerated. 13 Go test groups pass, build and vet clean. Not exercised against a live database or any of the third-party services. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 13 from CHANGES_.md. Three importable workflows plus the runbook for turning any of it on. The webhook verifies the HMAC signature before anything else touches the payload, in constant time, with a five-minute timestamp window. The endpoint is on the public internet and will be found; the signature is what stops it being fed fabricated reviews, and the window stops a captured payload being replayed forever. The classifier is pinned to an explicit model id, runs at temperature 0, and is constrained to a JSON schema whose decision field is an enum of exactly approve/reject/escalate. It never emits a free-form action string that something downstream parses loosely, and anything unparseable, out of enum, or missing a confidence becomes escalate. A pipeline that guesses when it cannot read its own input eventually publishes something nobody approved. The prompt states that everything inside the review tags is data, never instructions, and that an attempt to instruct the model is itself grounds to escalate. That is a mitigation, not the defence: the real defences are in Go, where an injection cannot argue with them. Links and contact details are rejected before any model call, and misconduct allegations escalate regardless of what the classifier concludes. Discord messages link to the authenticated queue and carry no decision token. A channel post is visible to everyone in it and is trivially forwarded, so a one-click approve link is a decision anyone can take. The runbook has the shadow-mode rollout, including the query that actually matters: how often the classifier said approve where a human said reject. Anything in that cell means auto-approve is not ready. Also documents that the Gemini free tier's terms permit Google to use submitted content to improve their products. That is disclosed on the privacy policy page, and the runbook notes it needs updating if you move to a paid tier. The sweep is scheduled here for completeness, but the runbook recommends Cloud Scheduler instead: an n8n outage should not also stop the email outbox draining. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
README.md covers the design and the rollout policy; this is the mechanical side -- generate three secrets, import three workflows, set five variables, add the Gemini credential, activate, point the API at it. Includes the curl that should FAIL. An unsigned POST to the webhook must be rejected: if it returns 200, HMAC verification is not running and anyone who finds the URL can feed the workflow fabricated reviews. That is the one check worth doing by hand. Notes that CLI imports do not activate workflows, which otherwise presents as every call 404ing with no obvious cause, and recommends Cloud Scheduler over the n8n schedule for the sweep so that an n8n outage cannot also stop the email queue draining. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three errors in the exported JSON, found by checking it against the actual node definitions rather than trusting the shape I wrote it in. `n8n-nodes-base.googleGemini` does not exist. The node is `@n8n/n8n-nodes-langchain.googleGemini`, resource `text`, operation `message`. `maxOutputTokens` defaults to 16. This was the dangerous one: 16 tokens truncates the JSON reply mid-object on essentially every call, and a truncated reply is unparseable, so the workflow would have escalated 100% of reviews while looking like a cautious classifier rather than a broken one. Set to 512. That failure survives a demo and gets discovered a month later from the queue depth. The messages array accepts only `user` and `model` roles, so the system prompt moves to `options.systemMessage`. Same content, different slot. The real change is to the guarantee. This node has a boolean `jsonOutput`, not schema-constrained decoding -- there is no `jsonSchema` option -- so nothing at the API boundary enforces that `decision` is one of three words. The README claimed it did. It now says plainly where the guarantee actually lives: the contract is stated in the system message, and `Parse decision` enforces it. That node is now load-bearing, so it is hardened: it reads the payload from any of the shapes the node emits, strips markdown fences, rejects out-of-enum decisions, coerces a non-numeric or out-of-range confidence to zero, and escalates an `approve` that arrives with no usable confidence -- which is a parse failure wearing a decision's clothes. Verified against twelve payload shapes, including the 16-token truncation: every valid shape parses, every malformed one escalates. Kept this design rather than switching to a Basic LLM Chain with a Structured Output Parser, and the README records why. A schema still admits a well-formed `approve` for a review that should be rejected, so it only rules out malformed output, which Parse already handles. The parser throws on violation, which routes to the error branch and leaves the review pending until the sweeper escalates it up to 30 hours later; the current path escalates immediately with a reason attached. Both fail safe, this one fails faster and says more. INSTALL.md gains the two settings that matter on that node and two new entries in the troubleshooting table. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
grade migration