sec: backfill legacy person_observations.titles into per-title rows - #217
Open
sroussey wants to merge 7 commits into
Open
sec: backfill legacy person_observations.titles into per-title rows#217sroussey wants to merge 7 commits into
sroussey wants to merge 7 commits into
Conversation
Replace the person_observation.titles array column with two tables: person_observation_title (one row per single title per observation, replaced wholesale on re-observation, reaped with the observation) and person_role (canonical-tier tenures: person + company CIK + one canonical title with a required start_date, optional end_date, and last_seen_date guard). EntityObserver records dated role assertions for claims carrying filing_date, source_filing_issuer_cik, and a role_scope population tag, canonicalizing titles through normalizeManagementTitles (compound titles split into separate tenures). Roster-complete filings — Form D related persons and the S-1 management section — close open tenures they no longer assert (closeUnassertedPersonRoles), with strict date guards so out-of-order replays are safe: an older roster never closes a newer assertion, a re-extraction that finds a person re-opens the tenure its own accession closed, and a departure and return yield two tenure rows. person_role rows are resolver-versioned like the address/phone junctions: purged by dropPrevious (person), rebuilt by extraction replays. New CLI query 'sec query person-roles <cik> [--current]'; 'sec query persons' joins titles from the child table. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VQVkDKxGfax1k9qbr2JPU
- Re-opening a tenure absorbs interposed return tenures so an extractor-version re-run cannot leave two overlapping open rows for one (person, title). - S-1 management closure is completeness-gated: sectionRunner now passes persist a meta.complete flag (true only when every extracted row survived the confidence floor and span verification), so a partial roster never records a false departure. - closeUnasserted re-reads each tenure and re-evaluates the date guard under the per-tenure lock, so a concurrent newer assertion always wins; a same-day assertion tie re-opens a closure; an out-of-order earlier roster tightens a closed tenure's end_date back to the first non-asserting filing. - A re-run of the accession that alone supported a tenure and no longer asserts it deletes the phantom row (equal-date self-retraction). - Tenure match keys clamp to the stored 256-char width so overlong titles cannot mint duplicates. - Form D roster closure requires at least one actually-observed person (a junk-only or all-entity related-persons list never mass-closes). - Replace literal NUL bytes in key-separator strings with \\x00 escapes so the source files are no longer binary to git. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VQVkDKxGfax1k9qbr2JPU
The DOM and node:stream/web ReadableStream types no longer overlap on a fresh dependency install, so the direct cast fails tsc (TS2352) on every branch. Route the cast through unknown, per the compiler's guidance — the runtime object a fetch body provides is exactly what Readable.fromWeb consumes. Pre-existing on the base branch; surfaced by CI's fresh install. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VQVkDKxGfax1k9qbr2JPU
Roster-closure completeness: S-1 closure requires every extracted row to survive filtering AND every persisted person to contribute an assertion (a row whose titles all filter away marks the group incomplete); Form D closure requires at least one observed person and no junk-dropped roster entry. A closure call with no recorded assertions, or an empty filing date, is a no-op — an empty asserted set must never read as 'everyone left'. Tenure semantics: a seated board role now also asserts the implied Director role, so gaining or losing a chairmanship never falsely closes a continuous directorship (the canonical title form drops the redundant bare Director). A second same-day accession asserting an open tenure is recorded as its latest supporter, so a re-extraction of the first cannot retract a tenure a sibling filing supports. The same-date reopen tie-break no longer erases a recorded gap (it yields to interposed return tenures), and absorbed rows are deleted only after the merged row is durable. Reaping now deletes tenures whose only support was the reaped filing (deleteSoleSupport) — assert-only scopes previously kept phantom roles forever. closeUnasserted counts only open-to-closed transitions. Mechanics: per-observation mutex + unchanged-list fast path + deleteSearch/ putBulk in the title repo; bulk delete in deleteForResolverVersion; parallel title/name joins in the CLI queries; strict CIK validation on person-roles; a resolver_version purge index (dropping the redundant company_cik one); eager alias-repo resolution; internal assertion-key kept out of the barrel; table-name doc fixes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VQVkDKxGfax1k9qbr2JPU
The title text is the row's identity within its observation — the same precedent as the junction tables keying on the associated value — so the storage layer now enforces one row per distinct title, and concurrent writers converge on the same rows via the PK instead of relying solely on the single-process mutex. title_index stays as a plain source-order column. replaceForObservation becomes a per-title diff: an unchanged title keeps its physical row across replays, a pure reorder only rewrites title_index in place, and only titles the new list no longer asserts are deleted — which also shrinks the mid-replacement crash window to genuinely removed titles. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VQVkDKxGfax1k9qbr2JPU
Filings order titles inconsistently, so the ordinal was dead weight. The title child row is now just its identity, (observation_id, title): a reordered replay is a complete no-op, replacement inserts only titles not already stored and deletes those no longer asserted, and reads sort by title text for determinism. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VQVkDKxGfax1k9qbr2JPU
An in-place DB upgrade past the person-title split leaves existing observations' titles trapped in the old `titles` JSON-array column on `person_observations`, while every read path now joins from the `person_observation_titles` child table — so titles silently disappear from query surfaces. This adds a `db setup` migration that copies each legacy row's array into per-title rows (empties dropped, case-insensitive dedupe, clamped to 256 chars) inside a single transaction, chunked and progress-logged. The legacy column is left in place (SQLite cannot drop it without a table rebuild) and the child-table PK plus INSERT OR IGNORE / ON CONFLICT DO NOTHING make the migration idempotent and safe when prior title rows already exist.
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
2 participants
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.
Summary
Follow-up to #216. That PR moves person titles from a JSON-array column on
person_observationsto a dedicatedperson_observation_titleschild table and switches every read path to join from the child. On any in-place DB upgrade past #216, existing observations' titles stay trapped in the old column and silently disappear from query surfaces.This PR adds a
db setupmigration that copies each legacy row's titles array into the child table (empties dropped, case-insensitive dedupe, clamped to 256 chars) inside a single transaction, chunked (5000 rows) and progress-logged every 100k rows. It runs fromsetupAllDatabasesimmediately after the child-tablesetupDatabase()and covers both the SQLite and Postgres backends. The legacytitlescolumn is left in place — SQLite cannot drop it without a full table rebuild, and no read path consults it any more.The migration is idempotent: the child-table PK
(observation_id, title)plusINSERT OR IGNORE/ON CONFLICT DO NOTHINGmeans replays skip rows already present, and pre-existing child-table rows for the same observation are preserved rather than overwritten. Dry-run bails at the top (raw SQL reaches around theReadOnlyTabularStoragewrapper).Merge order
Merge #216 first, then this PR — or squash both together. If this PR merges first (before #216), the migration is a no-op:
person_observationshas notitlescolumn and the SQLite / Postgres probes return cleanly.Verification
Ran in
/home/user/sec:bun test src/storage/observation/PersonObservationTitlesLegacyMigration.test.ts— 5 pass, 0 fail (populated → migrated; empty legacy → no-op; fresh DB with no titles column → no-op; idempotent second run; pre-existing child row preserved).bun test src/storage/observation/— 29 pass, 0 fail across 6 files.bun x tsc --noEmit -p .— clean.Only the SQLite branch is unit-tested; the Postgres branch is implemented but not covered. Recommend a follow-up PG integration test once a Postgres fixture harness is convenient.
Generated by Claude Code