Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 25 additions & 6 deletions cal/src/availability.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,19 @@

import { span } from "./trace.ts";

// The one shape this module traffics in: a half-open span of unix-ms. Busy
// intervals from the feed, expanded recurrences, held coffee slots and generated
// slots are all this, which is why it is exported rather than repeated inline.
// Every accumulator below needs it explicitly, because an empty array literal
// infers `never[]` under strictNullChecks: TypeScript only lets an array evolve
// from its pushes while that flag is off.
export type Interval = { start: number, end: number };

// The last-good calendar snapshot as stored in KV. `busy` is optional because
// every reader already defaults it, and that default is load-bearing rather than
// defensive: an older deploy's entry may predate the field.
type BusySnapshot = { ts?: number, busy?: Interval[] };

// ── calendar snapshot (stale-while-revalidate) ──────────────────────────────
// The ICS upstream can be slow or briefly down, and the old fetchBusy() gated
// the whole page on it AND returned [] on failure — which silently made EVERY
Expand Down Expand Up @@ -57,11 +70,17 @@ export async function fetchBusySWR(env, ctx, { allowStale = false } = {}) {
async function fetchBusySWRInner(env, ctx, allowStale, s) {
const kv = env.BOOKINGS;
const now = Date.now();
let snap = null;
let snap: BusySnapshot | null = null;
try { snap = kv ? await kv.get(BUSY_KEY, "json") : null; } catch (e) {
s.setAttribute("cal.snapshot_read_error", (e && e.message) || String(e));
}
const age = snap && Number.isFinite(snap.ts) ? now - snap.ts : Infinity;
// Read once into a local: `Number.isFinite` is not a type guard, so it cannot
// narrow `snap.ts` away from `number | undefined` and the subtraction below
// stays an error however the ternary is arranged. `!= null` does narrow, and
// the pair keeps the original meaning (a missing OR non-finite stamp is not an
// age, so both fall through to Infinity).
const ts = snap?.ts;
const age = ts != null && Number.isFinite(ts) ? now - ts : Infinity;
// Infinity is not a valid attribute value and "no snapshot" is not an age.
// Omitted rather than coerced, same discipline as the photo metadata.
if (Number.isFinite(age)) s.setAttribute("cal.snapshot_age_ms", age);
Expand Down Expand Up @@ -135,7 +154,7 @@ function parseICS(text) {
const windowStart = now - 2 * 86_400_000; // catch an in-progress recurrence
const windowEnd = now + RRULE_HORIZON_DAYS * 86_400_000;

const busy = [];
const busy: Interval[] = [];
let inEvent = false;
let dtstart, dtend, status, transp, rrule, exdates;

Expand Down Expand Up @@ -204,7 +223,7 @@ function expandEvent(dtstart, dtend, rrule, exdates, windowStart, windowEnd) {
const until = R.UNTIL ? parseICSDate(R.UNTIL, "") : null;
if (["DAILY", "WEEKLY", "MONTHLY", "YEARLY"].indexOf(freq) === -1) return [iv(dtstart)]; // unknown FREQ → base only

const out = [];
const out: Interval[] = [];
let n = 0; // occurrence index (counts toward COUNT even before the window)
// returns false when the series is exhausted (UTIL / COUNT / cap / past window)
const stop = (start) => (until && start > until) || (count != null && n >= count) || n >= RRULE_MAX_INSTANCES || start > windowEnd;
Expand Down Expand Up @@ -307,7 +326,7 @@ function icsZoneOffsetMinutes(ms, tzid) {
// conflating the two was a real bug: counting every calendar event toward the
// daily limit zeroed out availability for anyone with a busy calendar.
// returns [{start, end}, ...] in unix-ms.
export function generateSlots(env, busy, bookings = []) {
export function generateSlots(env, busy: Interval[], bookings: Interval[] = []) {
const tz = env.HOST_TIMEZONE;
const minNotice = +env.MIN_NOTICE_HOURS * 3600_000;
const lookahead = +env.MAX_LOOKAHEAD_DAYS * 86400_000;
Expand All @@ -334,7 +353,7 @@ export function generateSlots(env, busy, bookings = []) {
// existing coffee booking. limits, below, only count the coffee bookings.
const conflicts = [...busy, ...bookings];

const slots = [];
const slots: Interval[] = [];
const dayCounts = {}; // YYYY-MM-DD → count of coffee bookings (pending+confirmed)
const weekCounts = {}; // YYYY-WW → count

Expand Down
5 changes: 3 additions & 2 deletions cal/src/booking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,9 @@ export async function releaseSlot(env, b) {
// encoded in the key name, so this is one list() with no per-key gets.
/** @param {any} env @returns {Promise<Slot[]>} */
export async function listHeld(env) {
/** @type {Slot[]} */
const held = [];
// A REAL annotation. This was `/** @type {Slot[]} */`, which a .ts file
// ignores, so the array inferred `never[]` the moment strictNullChecks came on.
const held: Slot[] = [];
let cursor;
do {
const page = await env.BOOKINGS.list({ prefix: "held:", cursor });
Expand Down
2 changes: 1 addition & 1 deletion cal/src/email.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ function buildICS(env, booking) {
function foldLine(line) {
const enc = new TextEncoder();
if (enc.encode(line).length <= 75) return [line];
const out = [];
const out: string[] = [];
let cur = "", curBytes = 0, first = true;
for (const ch of line) { // iterate by code point, not UTF-16 unit
const chBytes = enc.encode(ch).length;
Expand Down
2 changes: 1 addition & 1 deletion cal/src/slots.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { listHeld } from "./booking.ts";
// ok, source). Callers that only need the listing destructure { slots };
// route_book also reads { cal } to fail closed on an unvouchable calendar.
// timings (optional) collects per-step durations for a Server-Timing header.
export async function listOpenSlots(env, ctx, timings = null, options = {}) {
export async function listOpenSlots(env, ctx, timings: Record<string, number> | null = null, options = {}) {
const mark = (name, p) => {
if (!timings) return p;
const s = Date.now();
Expand Down
6 changes: 5 additions & 1 deletion cal/src/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@
// its unit tests. That is correct: those tests assert booking policy, not
// telemetry, and fetchBusySWR is legitimately called with a null ctx there.

let tracing = null;
// The injected tracer, or null under plain node. Typed by the ONE method this
// module calls rather than by importing Cloudflare's shape: `cloudflare:workers`
// may not be imported here (gotcha 16), and the whole point of the injection is
// that this file never names that module.
let tracing: { enterSpan: (name: string, fn: (span: any) => any) => any } | null = null;

export function installTracing(candidate) {
// cal cannot import src/worker/lib/parse.ts: its Vitest pool boots from
Expand Down
12 changes: 10 additions & 2 deletions cal/test/availability.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,17 @@ const localParts = (ms) => {
timeZone: TZ, weekday: "short", hour: "numeric", hour12: false,
}).formatToParts(new Date(ms));
const wk = { Sun: 0, Mon: 1, Tue: 2, Wed: 3, Thu: 4, Fri: 5, Sat: 6 };
// `find` can miss, and the old `.value` on the result would have thrown a bare
// TypeError naming neither the part nor the formatter. Both parts are requested
// above, so a miss means Intl changed under us — worth saying so out loud.
const part = (type) => {
const hit = p.find((x) => x.type === type);
if (!hit) throw new Error(`Intl.DateTimeFormat produced no "${type}" part for ${TZ}`);
return hit.value;
};
return {
dow: wk[p.find((x) => x.type === "weekday").value],
hour: +p.find((x) => x.type === "hour").value % 24,
dow: wk[part("weekday")],
hour: +part("hour") % 24,
};
};

Expand Down
5 changes: 4 additions & 1 deletion cf-garage/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ export class Counter {
// in without ever clobbering a counter that's already live.
if (url.searchParams.has("seed")) {
if (!(await this.state.storage.get("seeded"))) {
n = parseInt(url.searchParams.get("seed"), 10) || 0;
// `get` answers `string | null` even behind the `has` above, and the old
// call leaned on `parseInt(null)` coercing to NaN for the `|| 0` to catch.
// `?? ""` reaches the same 0 by a route that is written down.
n = parseInt(url.searchParams.get("seed") ?? "", 10) || 0;
await this.state.storage.put("n", n);
await this.state.storage.put("seeded", true);
}
Expand Down
6 changes: 6 additions & 0 deletions config/tsconfig.cal-test.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@
"types": ["@cloudflare/workers-types"],
"resolveJsonModule": true,
"skipLibCheck": true,
// strictNullChecks ON. `strict` stays off: it bundles noImplicitAny,
// which is 7,197 diagnostics of annotation labour across this repo, and
// raising the two together buries the checks that find bugs under the
// ones that find missing annotations. This program is clean under it,
// and a contract test keeps the set of clean programs from shrinking.
"strictNullChecks": true,
"strict": false
},
// vitest.config.js is here deliberately. It is the file that decides which
Expand Down
6 changes: 6 additions & 0 deletions config/tsconfig.cf-garage-test.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@
"types": ["bun"],
"resolveJsonModule": true,
"skipLibCheck": true,
// strictNullChecks ON. `strict` stays off: it bundles noImplicitAny,
// which is 7,197 diagnostics of annotation labour across this repo, and
// raising the two together buries the checks that find bugs under the
// ones that find missing annotations. This program is clean under it,
// and a contract test keeps the set of clean programs from shrinking.
"strictNullChecks": true,
"strict": false
},
"include": ["../cf-garage/test/**/*.mjs"]
Expand Down
6 changes: 6 additions & 0 deletions config/tsconfig.cf-garage.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@
"types": ["@cloudflare/workers-types"],
"resolveJsonModule": true,
"skipLibCheck": true,
// strictNullChecks ON. `strict` stays off: it bundles noImplicitAny,
// which is 7,197 diagnostics of annotation labour across this repo, and
// raising the two together buries the checks that find bugs under the
// ones that find missing annotations. This program is clean under it,
// and a contract test keeps the set of clean programs from shrinking.
"strictNullChecks": true,
"strict": false
},
"include": ["../cf-garage/src/**/*.ts"]
Expand Down
6 changes: 6 additions & 0 deletions config/tsconfig.lens-reader-test.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@
"types": ["bun"],
"resolveJsonModule": true,
"skipLibCheck": true,
// strictNullChecks ON. `strict` stays off: it bundles noImplicitAny,
// which is 7,197 diagnostics of annotation labour across this repo, and
// raising the two together buries the checks that find bugs under the
// ones that find missing annotations. This program is clean under it,
// and a contract test keeps the set of clean programs from shrinking.
"strictNullChecks": true,
"strict": false
},
"include": ["../lens-reader/test/**/*.mjs"]
Expand Down
6 changes: 6 additions & 0 deletions config/tsconfig.lens-reader.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@
"types": ["@cloudflare/workers-types"],
"resolveJsonModule": true,
"skipLibCheck": true,
// strictNullChecks ON. `strict` stays off: it bundles noImplicitAny,
// which is 7,197 diagnostics of annotation labour across this repo, and
// raising the two together buries the checks that find bugs under the
// ones that find missing annotations. This program is clean under it,
// and a contract test keeps the set of clean programs from shrinking.
"strictNullChecks": true,
"strict": false
},
"include": ["../lens-reader/src/**/*.ts"]
Expand Down
6 changes: 6 additions & 0 deletions config/tsconfig.lwe-ask.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@
"types": ["@cloudflare/workers-types"],
"resolveJsonModule": true,
"skipLibCheck": true,
// strictNullChecks ON. `strict` stays off: it bundles noImplicitAny,
// which is 7,197 diagnostics of annotation labour across this repo, and
// raising the two together buries the checks that find bugs under the
// ones that find missing annotations. This program is clean under it,
// and a contract test keeps the set of clean programs from shrinking.
"strictNullChecks": true,
"strict": false
},
// A GLOB rather than a file list, so a new module joins the program by
Expand Down
5 changes: 4 additions & 1 deletion lens-reader/src/reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,10 @@ export function scoreExtraction({ source, kept, controls, title, markdown }) {
async function readCapped(response, maxBytes) {
if (!response.body) return "";
const reader = response.body.getReader();
const chunks = [];
// Annotated because an empty literal infers `never[]` under strictNullChecks:
// TypeScript only lets an array "evolve" from its pushes while that flag is
// off, so without this the push below assigns a chunk to `never`.
const chunks: Uint8Array[] = [];
let total = 0;
for (;;) {
const { done, value } = await reader.read();
Expand Down
26 changes: 21 additions & 5 deletions lens-reader/test/reader.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ test("read publishes Markdown from Readability's finished article node", async (
parseHTML(html).document,
{ charThreshold: 500 },
).parse();
// `parse()` answers null on a document it cannot extract. The old
// `baseline.content` would have thrown a bare TypeError there, blaming the
// assertion rather than the fixture.
if (!baseline) throw new Error("Readability extracted nothing from the fixture");
assert.equal(result.title, "Direct Article");
assert.equal(result.markdown, toMarkdown(baseline.content));
} finally {
Expand Down Expand Up @@ -187,13 +191,24 @@ test("the extractor actually extracts, which no other test here proved", async (
</body></html>`;

const result = new Readability(parseHTML(html).document, { charThreshold: 500 }).parse();
assert.ok(result, "extractor returned nothing at all");
// A THROW rather than `assert.ok`, which reads identically and does not narrow:
// TypeScript only applies an `asserts` signature when every name in the call
// target carries an explicit type, and `assert.ok` reached through a namespace
// import does not qualify. The four reads below are what needs the narrowing.
if (!result) throw new Error("extractor returned nothing at all");
// Both fields are independently nullable on Readability's result, and a null
// reaching `assert.match` fails on the argument type rather than on the thing
// this test is about. Named here so the failure says which field was missing.
const { title, content } = result;
if (title == null || content == null) {
throw new Error(`extractor returned no ${title == null ? "title" : "content"}`);
}
// Readability takes the title from <title>, not <h1>, so the fixture agrees
// on both rather than pinning which source wins.
assert.match(result.title, /The Real Article/);
assert.match(result.content, /genuine body prose/);
assert.doesNotMatch(result.content, /Advert copy/, "the aside survived extraction");
assert.doesNotMatch(result.content, /Subscribe now/, "a control label survived as prose");
assert.match(title, /The Real Article/);
assert.match(content, /genuine body prose/);
assert.doesNotMatch(content, /Advert copy/, "the aside survived extraction");
assert.doesNotMatch(content, /Subscribe now/, "a control label survived as prose");
});

test("the control census survives Readability mutation without a second DOM", async () => {
Expand All @@ -217,6 +232,7 @@ test("the control census survives Readability mutation without a second DOM", as
const labels = collectControlLabels(document);
const beforeBytes = document.body.innerHTML.length;
const result = new Readability(document, { charThreshold: 500 }).parse();
if (!result?.content) throw new Error("Readability extracted nothing from the control fixture");

assert.ok(document.body.innerHTML.length < beforeBytes / 2,
"Readability left the document intact, so the mutation boundary is no longer exercised");
Expand Down
6 changes: 5 additions & 1 deletion lwe-ask/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,11 @@ async function reindex(req, env) {
return metadata;
};

const BATCH = 25, vectors = [];
// Vectorize's upsert shape, stated because an empty literal infers `never[]`
// once strictNullChecks turns off evolving-array inference, and every push
// below would then be assigning to `never`.
const BATCH = 25;
const vectors: { id: any, values: any, metadata: ReturnType<typeof passageMetadata> }[] = [];
for (let i = 0; i < PASSAGES.length; i += BATCH) {
const chunk = PASSAGES.slice(i, i + BATCH);
const { data } = await env.AI.run(env.EMBED_MODEL, { text: chunk.map((p) => p.text) });
Expand Down
5 changes: 4 additions & 1 deletion src/worker/lib/crawl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,10 @@ export async function readResponseCapped(response, maxBytes = DEFAULT_CRAWL_BODY
: null;
if (!reader) return { text: "", bytesRead: 0, truncated: false, digest: "" };

const chunks = [];
// Annotated because an empty literal infers `never[]` under strictNullChecks,
// which the lens-reader Worker's program turns on: TypeScript only lets an
// array evolve from its pushes while that flag is off.
const chunks: Uint8Array[] = [];
let bytesRead = 0;
let truncated = false;
try {
Expand Down
57 changes: 57 additions & 0 deletions tools/contract-the-typescript-quarantine.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -415,3 +415,60 @@ test("a ramp step hands the remainder to the LARGEST incumbent", () => {
// Nothing serving yet: a first deploy has no remainder to hand out.
assert.equal(remainderHolder([], "7634b9d8-fc15"), null);
});

// ── strictNullChecks is a RATCHET, and it only ever tightens ────────────────
// `strict` is off everywhere and stays off: it bundles noImplicitAny, measured
// at 7,197 diagnostics across this repo against strictNullChecks' 1,544, and the
// two answer different questions. noImplicitAny finds missing annotations on
// code that was written without types. strictNullChecks finds places a null can
// actually arrive, which is the half worth having.
//
// The six programs below are clean under it and declare it. The list may only
// GROW: turning the flag off in one of them, or dropping a program from this
// list while its config still has it, fails here. That is the same mechanism
// config/ts-migration.json used for the Worker quarantine, pointed the other
// way — that one could only shrink, this one can only grow.
//
// TEXT-BASED on purpose. Proving a program is clean means RUNNING tsc against
// it, which is what `bun run typecheck` already does on every one of these; a
// second run per program here would put ~8 compilations on a suite that answers
// in about a second. What this asserts is the declaration, and typecheck is what
// makes the declaration true.
test("every program declared null-safe still declares it", async () => {
const { readdirSync } = await import("node:fs");

const DECLARED = [
"tsconfig.cf-garage.json",
"tsconfig.cf-garage-test.json",
"tsconfig.cal-test.json",
"tsconfig.lens-reader.json",
"tsconfig.lens-reader-test.json",
"tsconfig.lwe-ask.json",
].sort();

const configs = readdirSync(new URL("config/", ROOT).pathname)
.filter((f) => /^tsconfig\..+\.json$/.test(f) || f === "tsconfig.json");

// A FLOOR on the scan itself, because a test that read no configs would agree
// with an empty declaration list and report a clean run.
assert.ok(configs.length >= 8,
`only ${configs.length} tsconfigs found — this is reading the wrong directory`);

const strict = [];
for (const name of configs) {
const source = await readFile(new URL(`config/${name}`, ROOT), "utf8");
if (/"strictNullChecks"\s*:\s*true/.test(source)) strict.push(name);
}

const missing = DECLARED.filter((name) => !strict.includes(name));
assert.deepEqual(missing, [],
`these programs are declared null-safe but no longer set strictNullChecks:\n ${missing.join("\n ")}\n` +
"The flag comes off only by fixing whatever made it fail, never by editing this list down.");

// The other direction, so a program that gains the flag joins the record
// rather than sitting outside it: a config can be ahead of this list only by
// somebody forgetting to add it.
const undeclared = strict.filter((name) => !DECLARED.includes(name));
assert.deepEqual(undeclared, [],
`these programs set strictNullChecks but are not in the declared list: ${undeclared.join(", ")} — add them`);
});
Loading