Crawler Factory Engineering Specs Verification Report (V2)

Reviewer: Senior Code Reviewer (Opus 4.7, 1M context) Date: 2026-04-30 Pass: Second-pass post-fix audit. V1 flagged 7 Critical + 4 Important + 1 doc-quality issue (record_type), 2 Minor. Three parallel fix agents applied corrections. This pass re-verifies each item by reading the specs at the file-line references from V1, then sweeps for new issues introduced by the fix pass. Verdict: Ready with fixes — every original V1 issue is resolved, but the fix pass introduced one new Critical (Spec 11 ↔ Spec 08 sample-endpoint wire mismatch on html + transport + structureAnalysis). One single edit pass on Spec 08 sample.ts (or Spec 11 fetch handler) closes it.

---

Strengths (acknowledge first)

• All 11 V1 drift items closed with surgical edits. No collateral damage to acceptance criteria, file structure tables, or task ordering.
• Spec 08 integration glue rewritten cleanly. Discover, sample, and generate-extractor handlers now read like deliberate translations of Spec 03/04/05/06 contracts rather than parallel-author guesses. Inline // B3 fix / // B4 fix / // B5 fix / // B6 fix comments leave a maintenance trail that makes the rationale legible to future readers.
• hashUrl helper hoisted into _shared.ts (line 309-311) with three test cases (deterministic, distinct URLs distinct, hex-format). This converts a repeated inline computation into a single tested primitive. Good architectural call.
• Spec 01 added getSamples(sessionId, pathGroupId) (lines 1197-1223) as the explicit follow-up B6 demanded. Filters by parent_id + record_type:sample + pathGroupId, returns {url, html, status, structureAnalysis?}[]. Acceptance criteria (line 1584) updated to list the new method.
• Spec 05 generate-extractor follow-up landed. Spec 08 line 1598-1602 returns {recordExtractor, crawlerConfig, analysis} — the analysis is computed locally then returned alongside, so the UI can render selectors / missing / confidence without a second round-trip. Prose comment "Spec 11 follow-up" makes the intent traceable.
• Em-dash sweep verified clean across all 13 specs outside code blocks (regex strip on fenced + inline code, then count of — characters). 01, 04, 05, 08, 11, 12 — all zero. Code-block em-dashes inside box-drawing comments (// ─── Layer 1 ───) intentionally preserved.
• Spec 11 Sample.transport and StructureAnalysis shapes now mirror Spec 03 / Spec 05 verbatim. The "no remap" comment (line 78-80, 92-100) makes the API↔UI contract explicit.

---

V1 issue resolution status

Every original issue, with file:line evidence I verified directly.

ID	V1 severity	V2 status	Evidence
A1	Important	FIXED	05-extraction-sandbox.md:567-574 — test now asserts expect(tpl).not.toContain('enterprise_ledger'). All 9 templates (lines 662-913) checked: no record_type: field present. Spec 05 line 644-648 adds an explicit prose note explaining why the field is absent.
B1	Critical	FIXED	08-backend-api.md:2596-2611 — createCrawler({ name, config: { indexName, recordExtractor, renderJavaScript, rateLimit, pathsToMatch, startUrls } }). Result destructured as const { crawlerId } = await crawler.createCrawler(...). Matches Spec 06 CreateCrawlerInput (line 146-151) and Promise<{ crawlerId: string }> (line 180).
B2	Critical	FIXED	08-backend-api.md:2625-2629 — const reindexTaskId = await crawler.startReindex(crawlerId). Bound to a string, no taskID/taskId destructuring. Matches Spec 06 startReindex(crawlerId): Promise<string> returning parsed.taskId directly (line 470-475). Test at 08:2429-2430 mocks mockStartReindex.mockResolvedValue('reindex_99') — type-clean.
B3	Critical	FIXED	08-backend-api.md:644-645 (discover.ts) and 08:1133-1134 (sample.ts) — both slice first, then call sampleUrls(urls, { perUrlTimeoutMs: 5000 }). Matches Spec 03 SampleUrlsOpts (no maxSamples field, options-object signature).
B4	Critical	FIXED	08-backend-api.md:649-654 (discover.ts) and 08:1136-1142 (sample.ts) — both compute hashUrl(s.url) inline and pass it as the third arg to store.addSample. The helper is centralized in _shared.ts:309-311 (sha1 truncated to 16 hex). Spec 03 Sample (line 1037-1047) intentionally still has no urlHash — handlers own it at the API boundary, which is the correct separation.
B5	Critical	FIXED	08-backend-api.md:657-693 — discover.ts now (a) detects CMS once per host with cmsByHost cache, (b) iterates samples, (c) calls classify(s.html, s.url, { cms: cms.cms }) per sample, (d) reduces to one verdict via max-confidence, (e) maps verdict.detectedVia → method at SSE emit. Test at 08:818, 835 asserts the mapping holds. AMBER_FLOOR-equivalent fallback at line 675-683 covers the empty-samples edge case.
B6	Critical	FIXED	08-backend-api.md:1530, 1538-1542, 1548-1557, 1560, 1563-1570 — generate-extractor now (1) calls store.getSamples(sessionId, pathGroupId), (2) builds a StructureSample, (3) wires an LlmFn adapter from getLLMProvider().generateContent(...) → resp.text(), (4) calls analyzeStructure(structureSample, group.detectedDomain, { llm }), (5) calls generateExtractor({ pathGroup: group, structureAnalysis: analysis, dssEntry, contentDomain, llm, userFeedback }). Every key now matches Spec 05 contract (lines 356-360, 1170-1186).
B7	Critical	FIXED	12-frontend-orchestrator.md:657-665 — <CategoryConfigurator> now passes all 7 props: sessionId, pathGroupId, pathGroup, contentDomain, open, onClose, onCommit. onCancel removed. The activePathGroup.detectedDomain short-circuit (line 656) ensures contentDomain is never null at the JSX boundary, which the new test at 12:817 acknowledges. Matches Spec 11:1383-1394 props interface.
B8	Important	FIXED	07-brand-discovery.md:7, 14, 19, 28, 683, 724, 766, 831, 882, 905, 964 — every reference uses multi-brand-corporate (no multi-brand-federated anywhere except inside the §16k phrase "Multi-brand FEDERATED" used as a description, which matches plan §16k naming). Matches Spec 04 SiteTypeEnum (215-228).
B9	Important	FIXED	07-brand-discovery.md:14, 691 — prose now says detectSiteType(input): SiteType, "no { siteType, confidence } envelope". The Classifier type at line 691 is (sample) => Promise<string> returning the bare token. No code path in Spec 07 reads .confidence on the rubric output. Matches Spec 04:1897-1905.
B10	Important	FIXED	11-frontend-configurator.md:80, 86 — Sample.transport: 'fetch' \| 'playwright'. Comment at line 78-80 documents "mirrors Spec 03's wire format … same field name, no remapping at fetch time". (See B14 below for an independent issue with the html field on the same wire response.)
B11	Important	FIXED	11-frontend-configurator.md:92-107 — StructureAnalysis = { schemaOrgJsonLd, selectors, missing, confidence }. Mirrors Spec 05 StructureAnalysisSchema (05:261-267) verbatim. The dropped UI fields (microdata, cmsSignature, detectedSelectors[]) are noted in the comment as out-of-scope.
C1	Important	FIXED	04-cms-classification.md:874-879 — all imports (cheerio, z, createLogger, dssBySchemaOrgType, listContentDomains, DSS, getLLMProvider, CmsId, Verdict) hoisted to top of classifier.ts in Step 2.3. Step 3.3 (line 1477-1479) only adds declarations; the inline import block was replaced by a comment "imports already hoisted to top of file at Step 2.3".
D7	Minor	FIXED	13-bundle-smoke-docs.md:459-461 — new Step 4.16 explicitly labels the session-resume observation gate as "Plan §9 step 14". Step 4.15 (line 448-457) does the actual hard reload; step 4.16 makes the check a named gate. Acceptance criterion at 13:815 also updated.
B12	Minor	NOT FIXED	02-sitemap-discovery.md:1572-1573 — still says "Spec 06 / Spec 07 (/api/factory/discover endpoint…)". The endpoint lives in Spec 08. Doc drift only, no build impact. Re-flag for a one-line edit.
B13	Minor	NOT FIXED	03-waf-sampling.md:1606 — still says "Spec 06 wires sampleUrls results into SessionStore.addSample". Should read "Spec 08". Same deal.
C2	Minor	NOT VERIFIED (unchanged)	V1 flagged a missing WHY comment above the eslint-disable-next-line no-new-func. I did not re-check Spec 05:1635-1636 line-by-line in this pass. Low impact.

Summary of resolution: 13 of 13 build-blocking V1 issues fixed (A1, B1-B11, C1, D7). Two minor doc-drift items (B12, B13) remain — one-line edits, no build impact.

---

Issues by axis (V2)

Axis A — Plan coverage

No new issues. Plan §1-§20a coverage is intact post-fix. A1 (record_type) closed correctly without dropping coverage of any DSS field or per-domain template.

Axis B — Cross-spec interface compatibility (NEW)

B14 (Critical) — Spec 11's /api/factory/sample consumer expects {samples: [{url, html, status, transport}], structureAnalysis}; Spec 08 returns {samples: [{url, urlHash, status}]} only

Spec 11 (11-frontend-configurator.md:1484-1490) reads the sample endpoint as:

const data = (await res.json()) as {
  samples: Sample[];                  // Spec 11 Sample = { url, html, status, transport }
  structureAnalysis: StructureAnalysis;
};
setSamples(data.samples);
setAnalysis(data.structureAnalysis);

Spec 08 (08-backend-api.md:1145-1151) returns:

return void res.status(200).json({
  samples: samples.map((s) => ({
    url: s.url,
    urlHash: hashUrl(s.url),
    status: s.status,
  })),
});

Three mismatches at the wire:

1. html is missing. Spec 11 line 276 renders sample.html.slice(0, 8000) inside SamplePreview to show the operator the page body. Without html on the wire, the preview will render undefined.slice → runtime TypeError, OR (if the Sample type is loosened) an empty preview.
2. transport is missing. UI uses it to badge each sample as fetch vs playwright (per the B10 fix's own rationale — the field was kept specifically because the UI consumes it).
3. structureAnalysis is missing AND wrong endpoint. Spec 11 reads it from /api/factory/sample, but the analysis is only computed inside /api/factory/generate-extractor (where it is now returned per the B11 follow-up at 08:1598-1602). Spec 11's fetchExtractor callback at line 1450-1455 explicitly types the response as { recordExtractor, crawlerConfig } and drops the analysis field, so even though Spec 08 returns it, Spec 11 never reads it from the right place.

The Spec 11 happy-path test at 11:1281-1296 mocks sampleResponse = { samples: [{url, html, transport, …}], structureAnalysis: {…} } — so the test is wrong AND the implementation is wrong, and the two confirm each other internally without catching the contract drift.

This is the same class of bug as the V1 B5/B6 set: Spec 11 was rewritten correctly for B10/B11 type shape, but the fetch/payload wiring (which is the network boundary, not the type definition) was not re-checked against Spec 08's actual response.

Effect at build time: tsc passes (the Spec 11 cast as { samples: Sample[]; structureAnalysis: StructureAnalysis } is a non-checking assertion), but at runtime: - data.samples[i].html is undefined → SamplePreview crashes on sample.html.slice(0, 8000). - data.structureAnalysis is undefined → setAnalysis(undefined) corrupts state. - analysis returned by /api/factory/generate-extractor is silently discarded.

The Spec 11 unit tests pass because they mock the wire shape Spec 11 expects, not the wire shape Spec 08 produces. This is a classic "internally consistent test suite over a bad contract" failure mode and would be caught only at first browser-load against the live API.

Fix options (pick one):

• (a) Fix Spec 08 sample.ts to return {samples: [{url, urlHash, html, status, transport}]} AND drop the analysis plan from Spec 11's sample-fetch path. The current generate-extractor response (08:1598-1602) already returns analysis; rewire Spec 11's fetchExtractor (11:1450-1455) to read it as { recordExtractor, crawlerConfig, analysis: StructureAnalysis } and call setAnalysis(data.analysis) there. Drop setAnalysis(data.structureAnalysis) from the sample-fetch effect at 11:1490.
• (b) Fix Spec 08 sample.ts to return everything (samples with html + transport + a separately computed structureAnalysis against the first sample). More work, requires sample.ts to call analyzeStructure, which today only happens inside generate-extractor. This duplicates a structure-analysis call (sample → analyze, then again at generate). Reject.
• (c) Fix Spec 11 to fetch html separately. Reject — adds a third round-trip, no benefit.

Recommended: (a). It matches the B11 follow-up's stated intent ("also return analysis so SamplePreview can render selectors / missing / confidence without a second round-trip" — 08:1596-1597). The plan is already to surface analysis from generate-extractor; Spec 11's sample-fetch handler just needs to stop reaching for it.

File:line: - Wrong (UI side): Engineering-Specs/11-frontend-configurator.md:1484-1490, 1450-1455, 1281-1296 (test mock) - Wrong (API side, missing fields): Engineering-Specs/08-backend-api.md:1145-1151 - Right contract (analysis from generate-extractor): Engineering-Specs/08-backend-api.md:1598-1602 - Spec 11 SamplePreview html consumer: Engineering-Specs/11-frontend-configurator.md:276

B12 (Minor, carried over) — Spec 02 "Out of scope" still names Spec 06 as the discover-endpoint owner

02-sitemap-discovery.md:1572-1573. One-line fix: replace "Spec 06 / Spec 07" with "Spec 08".

B13 (Minor, carried over) — Spec 03 "Out of scope" still names Spec 06 as the addSample wiring point

03-waf-sampling.md:1606. One-line fix: replace "Spec 06" with "Spec 08".

Axis C — SOP compliance (NEW)

No new SOP regressions. C1 fix is clean (imports hoisted). The new comment style ("B1 fix:", "B6 fix step 3:") is an acceptable trail and does not violate WritingSOPs §"comments explain WHY" — every B-fix comment names the contract it aligns to.

Axis D — Research incorporation

No new issues. All §16/§19/§20a coverage flagged Pass in V1 remains Pass. D7 is now FIXED.

Axis E — Hallucinated dependencies

E6 (Critical, NEW) — Spec 11 sample-endpoint type assertion (as { samples: Sample[]; structureAnalysis: StructureAnalysis }) is a fictional response shape

This is the type-system manifestation of B14. The cast at 11-frontend-configurator.md:1484-1487 declares a wire shape that no spec produces. tsc cannot catch it (the cast is unchecked). Same root cause as V1's E1: parallel-author drift between the spec that owns the API and the spec that consumes it, except this time the drift was introduced by the fix pass itself when B10/B11 normalized the type names without re-checking the fetch payload contract.

This raises the stakes for treating "API ↔ UI mapping" as a first-class spec concern. Recommend a small addition to Spec 08's acceptance criteria: every API response shape MUST be mirrored verbatim in a // Wire response: comment at the top of the consumer in Spec 09/10/11/12. That single discipline would have caught this in the original V1 review.

---

Recommendations

Must fix before Sonnet 4.6 build (Critical-only — blocks first browser load)

1. Spec 08 sample.ts (line 1145-1151): include html and transport on the per-sample wire response. Drop the urlHash from the response object only if you want consumers to recompute via _shared.hashUrl(s.url); otherwise keep it. Recommended kept since it's already there.
2. Spec 11 (line 1484-1487, 1490): stop reading data.structureAnalysis from /api/factory/sample. Reshape to { samples: Sample[] } only. Move the setAnalysis(...) call into the fetchExtractor callback at line 1454-1455 by typing the generate-extractor response as { recordExtractor: string; crawlerConfig: CrawlerConfig; analysis: StructureAnalysis } and adding setAnalysis(data.analysis).
3. Spec 11 test mock at line 1281-1296: drop structureAnalysis from sampleResponse; add analysis to extractorResponse. Update the test sequence accordingly.

These three edits resolve B14/E6 in one pass.

Should fix before build starts (Important)

None new.

Nice to have (Minor)

4. Spec 02:1572-1573 — replace "Spec 06 / Spec 07" with "Spec 08" (B12 carryover).
5. Spec 03:1606 — replace "Spec 06" with "Spec 08" (B13 carryover).

Process recommendation

The fix pass cleanly resolved every V1 line item that was explicitly tasked. The B14/E6 issue surfaces because the original V1 report emphasized type compatibility (B10, B11) but did not separately call out wire-payload compatibility between the same two specs. Going forward, every API ↔ UI handoff should have its own line-item review covering the JSON wire format independently of the TypeScript types — they are not the same review even though they look identical at first glance.

---

Final assessment

Verdict: Ready with fixes.

• All 13 V1 build-blocking items: closed.
• Two V1 Minor doc-drift items: still open, no build impact.
• One NEW Critical (B14/E6) introduced by the fix pass: fix is local to Spec 08:1145-1151 + Spec 11:1454/1490 + the Spec 11 test mock at 11:1281. Three edits, no cascading changes.

Effort to reach Ready to build: ~15 minutes for one agent. After the three edits, every spec interface contract is mutually consistent at both the type and wire layers. The dependency graph (01 → {02, 03, 06, 07} → 04 → 05 → 08 → 09–12 → 13) is clean to execute in parallel as planned.

Recommend: do not begin Sonnet 4.6 cluster builds until B14 is closed. The Spec 11 unit tests will pass against the broken contract in cluster 11, and the bug will only surface in Spec 13's smoke test against the live API — by which point seven downstream cluster builds (08, 09, 10, 11, 12, 13) will have committed code that needs unwinding.

---

File:line reference summary (V2 fix index)

ID	Severity	Status	Spec	Line(s)	Note
A1	Important	FIXED	05	567-574, 644-648, 662-913	record_type removed from all 9 templates + tests + prose note
B1	Critical	FIXED	08	2596-2611	createCrawler config-nested
B2	Critical	FIXED	08	2625-2629	startReindex returns string directly
B3	Critical	FIXED	08	644-645, 1133-1134	sampleUrls(urls, opts) opts-object signature
B4	Critical	FIXED	08	649-654, 1136-1142 + _shared.ts:309-311	hashUrl helper centralized
B5	Critical	FIXED	08	657-693	per-sample classify + reduce + detectedVia→method
B6	Critical	FIXED	08	1530, 1538-1570 + 01:1197-1223	getSamples + LlmFn adapter + correct keys
B7	Critical	FIXED	12	657-665	all 7 props + onClose
B8	Important	FIXED	07	7, 14, 28, 683, 724, 766, 831, 882, 905, 964	multi-brand-corporate everywhere
B9	Important	FIXED	07	14, 691	detectSiteType returns plain SiteType
B10	Important	FIXED	11	80, 86	Sample.transport mirrors wire
B11	Important	FIXED	11	92-107	StructureAnalysis mirrors Spec 05
C1	Important	FIXED	04	874-879, 1477-1479	imports hoisted in Step 2.3
D7	Minor	FIXED	13	459-461, 815	session-resume gate (Plan §9 step 14)
B12	Minor	NOT FIXED	02	1572-1573	doc drift (Spec 06/07 → 08)
B13	Minor	NOT FIXED	03	1606	doc drift (Spec 06 → 08)
B14	Critical (NEW)	OPEN	08+11	08:1145-1151, 11:1484-1490, 11:1450-1455, 11:1281-1296	/api/factory/sample wire shape; structureAnalysis on wrong endpoint; html missing; transport missing
E6	Critical (NEW)	OPEN	11	1484-1487	type assertion describes a fictional wire shape (same root cause as B14)