Search Intelligence phase 2 — keyword gap, rank tracking, share of search - #193
Closed
mattia-chksg wants to merge 10 commits into
Closed
Search Intelligence phase 2 — keyword gap, rank tracking, share of search#193mattia-chksg wants to merge 10 commits into
mattia-chksg wants to merge 10 commits into
Conversation
Fork of every-app/open-seo pinned to v0.1.3 (9d19e43), operated as the Morgana Search Intelligence Engine. Phase 0 imports no SEO functionality: it establishes an isolated, observable, zero-spend staging engine. The patch series is deliberately small — three upstream files touched, +72/-3 lines, each marked with a MORGANA LOCAL PATCH comment. No upstream file is renamed, restructured, rebranded or deleted; capabilities are disabled through configuration and guards rather than by removing code, so future upstream merges stay clean. P1 wrangler.staging.jsonc + a one-line vite configPath hook. The staging config drops upstream's */15 cron (it runs billable rank checks), disables traces (spans bill as observability events), drops Smart Placement, and sets workers_dev/preview_urls false with no route. P2 /healthz, /readyz and /internal/status, handled before upstream dispatch so a probe performs no other work and cannot trigger the self-host telemetry heartbeat. P3 DATAFORSEO_SEARCH_INTELLIGENCE_API_KEY alias, so this engine can never use Brand Monitoring's DataForSEO credential or balance. A DataForSEO subaccount is a secret change with no code change. P4 Zero-spend guard that refuses to boot if paid calls are enabled against a zero cap, an explicit MCP kill-switch (MCP is reachable in cloudflare_access mode, so no-route alone is not a control), a scheduled-run refusal, structured logging with redaction, a separate dataforseo_search_intelligence cost centre, and a resource isolation guard that refuses any Morgana production name or id. P5 UPSTREAM.md, THIRD_PARTY_NOTICES.md and a read-only upstream drift reporter. MIT licence and upstream copyright preserved. Security posture: the Worker has no public ingress at all. That is the compensating control for the absent Cloudflare Access application, which could not be created because the available token lacks Zero Trust scope. AUTH_MODE=cloudflare_access with TEAM_DOMAIN and POLICY_AUD unset makes every application route fail closed as a second, independent layer. Verified: 768 upstream tests pass unchanged, 31 new tests, typecheck clean, 37 migrations applied to a fresh local D1 and to the dedicated staging D1, and the deployed workers.dev hostname returns Cloudflare error 1042 (no route) rather than the application.
First slice of Search Intelligence phase 1: the persistence shape and the pure logic it will be built on. No provider call and no API surface yet. Seven tables in both dialects (schema-parity guards the Postgres mirror, which db:generate does not regenerate). Migration 0037 is purely additive — seven CREATE TABLE, zero destructive statements. Keywords and pages hang off the SNAPSHOT rather than the entity so a historical state can be reconstructed exactly, and domain_snapshots carries a UNIQUE dedupe key so a retry, a concurrent manual refresh, a duplicated tick and a provider timeout-after-success cannot produce two snapshots for one day. Domain validation wraps upstream's normalizeDomainInput rather than replacing it, but tightens it where the threat model differs: upstream is fed by a product form and is forgiving by design, silently dropping URL credentials and accepting any scheme://. An admin API must refuse those instead — that is how an SSRF-shaped input becomes stored configuration. Also rejected: IP literals, loopback, private suffixes, and a bare domain carrying a path (ambiguous — refuse rather than guess). Internationalised domains are stored and displayed as ASCII punycode with a flag, deliberately not prettified to Unicode: rendering the Unicode form is precisely what makes a homograph indistinguishable from the real thing. The governing rule in metrics.ts is that a missing value is never a zero. Deltas distinguish not_available (no current value — a provider gap) from insufficient_history (no baseline — a young series that heals itself), and Estimated Organic Visibility Share refuses to compute at all when a compared domain has no estimate, when markets differ, when snapshots are not contemporaneous, or when the total is zero. A confidently wrong comparison table is worse than an empty one. Budget is a second, fully independent guard from Brand Monitoring's, with the decision-every-app#84 lesson built in from day one: free lifecycle calls count as requests but never as metered, so they can never ration paid work. A zero cap means "cannot spend", never "unlimited", and is evaluated independently of the paid-calls flag so flipping one flag cannot by itself authorise spending. The isolation guard gains a migration preflight: the config can be correct while the CLI argument is wrong, since `d1 migrations apply <name>` takes the database from the argument. Verified to reject the Morgana production database, a non-staging target, and a config/argument mismatch. 51 new tests; one caught a real bug where the root path normalised to `example.com/`, which would have keyed one page two ways.
Completes the engine side of Search Intelligence phase 1: the store, the fixture provider, the orchestration layer and the versioned Service Binding surface at /internal/si/*. Idempotency is enforced by the database rather than by a prior read. Two concurrent refreshes both attempt the snapshot insert and exactly one wins on the UNIQUE dedupe key; the loser returns the winner's id instead of an error, so a manual refresh racing the scheduler sees the snapshot the scheduler just produced rather than paying for a second one. Refresh jobs collapse the same way, so a duplicated tick, a double-clicked button and a retry become one job instead of three billable calls. Fixtures exist because there is no credential and no usable DataForSEO sandbox. They are deterministic (hashed from domain and date, so tests assert stable numbers) and unmistakable: every row is stamped source="fixture", and fixture mode is only ever selected when paid calls are OFF. The engine must never serve synthetic numbers to an operator who asked for real ones. A no-data domain is included on purpose so the "metric unavailable" path is exercised rather than assumed. Live collection is deliberately left as an explicit refusal rather than a half-written provider call. The branch is unreachable in every deployed configuration today, and whoever adds the credential should get a clear "not implemented" instead of a silent partial snapshot. The API is dispatched through the existing private Phase-0 entry point so the engine keeps exactly one ingress, and the whole surface sits behind SEARCH_INTELLIGENCE_ENABLED — with it off the engine behaves as though the feature does not exist, which is what makes the Morgana kill switch total rather than cosmetic. The import of the SI module is dynamic, not static. A static one pulled the Drizzle/D1 layer — and therefore cloudflare:workers — into the eager graph, which broke the Node-side Phase-0 tests and would have made /healthz load the database stack it is explicitly meant not to touch. Verified: 878 upstream tests pass, typecheck clean, the lean-bundle guard is satisfied, and 38 migrations apply to a fresh local D1. One pre-existing upstream flake observed once and NOT masked: src/server/lib/dataforseo/client.test.ts "skips billing in non-hosted mode" failed in one full-suite run and passed in isolation and in two subsequent full runs. It exercises the 250ms retry backoff with real timers, is upstream code untouched by this change, and is recorded in the phase-1 report rather than papered over with a retry.
Turns SEARCH_INTELLIGENCE_ENABLED on for the staging engine only, so the private /internal/si/* API answers and the integration can be verified end to end. This cannot cause spending: paid calls stay off, both caps stay zero, and no DataForSEO credential exists, so the collector resolves to deterministic fixtures. Morgana's production config keeps every flag false and its env validation refuses to let these be true in production.
CI failed on knip, the repo's dead-code gate. Fixed rather than configured around: 43 symbols were exported that nothing outside their own module imports, so the export was surface without a consumer. Four functions were genuinely dead, not merely over-exported: addSpend, recordProviderFailure, recordProviderSuccess and recordAlertThreshold write the budget state, and nothing calls them because live collection is not implemented in phase 1. Shipping them would be unreachable code that knip is right to reject, and keeping them "for phase 2" is exactly the habit the gate exists to break. They are removed with a comment recording that the budget READ path is used and tested while the WRITE path belongs with the live collector. Note that knip had never seen the Morgana patch series before: the phase-0 branch was pushed without a PR, so CI never ran on it. That is why some of the flagged exports date from phase 0 rather than this phase. knip clean, typecheck clean, 878 tests pass.
Removing 'export' and deleting the dead budget writers changed line wrapping in phase0-guard.ts and store.ts. Formatting only; no behaviour change, and knip, typecheck and the Morgana tests all still pass.
CI's oxlint --type-aware stage reported 25 errors that never appeared locally, because the local ci:check stopped earlier on the Windows CRLF false alarm. Fixed at the root rather than by relaxing any rule. Type assertions removed by making the types true instead of asserting them: drizzle's $inferSelect replaces hand-written row interfaces, the projection helpers take typed parameters instead of unknown, readJson uses a type predicate so the check and the narrowing are one statement, and the counter snapshot is written as an explicit literal — adding a counter to the union without adding it there is now a build error rather than a silent zero. Three modules exceeded the 400-line limit and dispatch had a complexity of 62 against a ceiling of 40 with six parameters against five. Split along real seams rather than arbitrary cut points: ledger-store (accounting) and cost (the read side of the ledger) leave store and service; projections (what we expose), http (response shaping) and router (the route table) leave api, which keeps only the posture — feature gate, logging, and turning any failure into a sanitized response. dispatch now takes one context object and splits into reads and operations, so "what can change state" is a readable list instead of a branch buried in a 60-way conditional. Also: toSorted over sort, Array.from over a useless spread, and the last unused exports removed. Local gate now matches CI: prettier, knip, tsc, oxlint --type-aware all clean, 878 tests pass.
This fork is public, and the isolation guard was carrying Morgana's production D1 UUID and resource names in plaintext, while a redaction test used a real employee email address. Both are on the Phase-0 do-not-publish list and both have been public since the phase-0 push. The guard only ever needs to answer "is this value forbidden", which a hash answers just as well, so the denylist is now truncated SHA-256 digests. The test address becomes person@example.com. Neither is a credential — a D1 UUID grants no access without account auth — but they are internal infrastructure identifiers and a real person's address in a public repository, which is reason enough. Verified the guard still passes the staging config and still rejects Morgana's real wrangler.jsonc. 82 tests, lint clean.
Adds the Phase-2 Search Intelligence core to the private staging engine: keyword clusters, a tracked-keyword watchlist, rank observations, keyword gap classification, Tracked Keyword Share of Search, ranking events and a Phase-2 cost ledger. Eight new tables in both dialects (migration 0038 / pg 0015), all additive. Notable design choices: - `si_rank_snapshots`, not `rank_snapshots`. Upstream already owns that name; sharing it made drizzle silently drop our table from the migration while our export won the runtime barrel, so writes would have landed in upstream's table. - Losses require confirmation, gains do not. A single bad observation is usually SERP noise, so a drop is only announced when the observation before the previous one was healthy. Competitive events fire for `critical` keywords only. - A missing value is never a zero: share of search refuses to report below 50% volume coverage and returns `insufficient_data` instead. - Dedupe keys are UNIQUE constraints, so replaying a tick is free. - `dueKeywords` encodes the budget policy in SQL priority ordering; the scheduler cannot accidentally spend on low-priority terms first. - `ranking-events/ack` exists because Morgana owns delivery. Without an acknowledgement round-trip the same event is handed out every tick. Still fixture-only: `runRankTick` refuses `live` explicitly, since the engine has no DataForSEO credential and must never reach for Morgana's.
`phase2CostStatus` reported `dataforseo_search_intelligence`, the same string phase 1 uses, so the two ledgers were indistinguishable by cost centre — which defeats the point of keeping them separate. "What is rank tracking costing us" should be answerable directly, not by subtracting one ledger from another. Found by the service-binding verification run against the deployed staging engine, not by a unit test.
Author
|
Opened against the wrong repository by mistake — this belongs on our fork. Closing immediately. |
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.
Engine side of Phase 2. Eight new tables in both dialects (migration
0038/pg
0015, all additive), the keyword/cluster stores, gap classification,Tracked Keyword Share of Search, ranking-event detection and a Phase-2 cost
ledger with its own cost centre.
Still fixture-only:
runRankTickrefuses aliveprovider explicitly, sincethis engine has no DataForSEO credential and must never reach for Morgana's
Brand Monitoring one.
Notable
si_rank_snapshots, notrank_snapshots. Upstream already owns thatname; sharing it made drizzle silently drop our table from the migration
while our export won the runtime barrel, so writes would have landed in
upstream's table.
event when the observation before the previous one was healthy — a single bad
observation is usually SERP noise. Competitive events fire for
criticalkeywords only.
insufficient_datarather than a misleading number.ranking-events/ackexists because Morgana owns delivery; without anacknowledgement round-trip the same event is handed out every tick.
Verification
typecheck, oxlint
--type-awareand knip clean; 126 tests insrc/server/morgana; 180 schema-parity assertions; migration applied to thestaging D1 (25 commands, 0 destructive); deployed and verified over a real
Service Binding with no public ingress and zero spend.