ENG-2155 Migrate existing database titles to undecorated core_title #1332
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import { | ||
| buildCoreTitleBackfill, | ||
| mergeNodesBySourceLocalId, | ||
| } from "../coreTitleBackfill"; | ||
| import type { RoamDiscourseNodeData } from "../getAllDiscourseNodesSince"; | ||
|
|
||
| const node = (sourceLocalId: string): RoamDiscourseNodeData => ({ | ||
| author_local_id: "author", | ||
| author_name: "Author", | ||
| source_local_id: sourceLocalId, | ||
| created: "1", | ||
| last_modified: "2", | ||
| text: `CLM - ${sourceLocalId}`, | ||
| type: "claim-type", | ||
| }); | ||
|
|
||
| describe("buildCoreTitleBackfill", () => { | ||
| it("forces in local nodes whose row has no core_title", () => { | ||
| const backfill = buildCoreTitleBackfill({ | ||
| conceptRows: [ | ||
| { source_local_id: "a", core_title: null }, | ||
| { source_local_id: "b", core_title: "already set" }, | ||
| ], | ||
| localNodes: [node("a"), node("b")], | ||
| }); | ||
|
|
||
| expect(backfill.nodesToBackfill.map((n) => n.source_local_id)).toEqual([ | ||
| "a", | ||
| ]); | ||
| expect(backfill.withCoreTitleCount).toBe(1); | ||
| expect(backfill.orphanedCount).toBe(0); | ||
| }); | ||
|
|
||
| it("reports rows that are no longer in the graph as orphaned", () => { | ||
| const backfill = buildCoreTitleBackfill({ | ||
| conceptRows: [ | ||
| { source_local_id: "a", core_title: null }, | ||
| { source_local_id: "gone", core_title: null }, | ||
| ], | ||
| localNodes: [node("a")], | ||
| }); | ||
|
|
||
| expect(backfill.nodesToBackfill.map((n) => n.source_local_id)).toEqual([ | ||
| "a", | ||
| ]); | ||
| expect(backfill.orphanedCount).toBe(1); | ||
| }); | ||
|
|
||
| it("skips rows without a source_local_id", () => { | ||
| const backfill = buildCoreTitleBackfill({ | ||
| conceptRows: [{ source_local_id: null, core_title: null }], | ||
| localNodes: [node("a")], | ||
| }); | ||
|
|
||
| expect(backfill.nodesToBackfill).toEqual([]); | ||
| expect(backfill.withCoreTitleCount).toBe(0); | ||
| expect(backfill.orphanedCount).toBe(0); | ||
| }); | ||
|
|
||
| it("is a no-op once every row has a core_title", () => { | ||
| const backfill = buildCoreTitleBackfill({ | ||
| conceptRows: [ | ||
| { source_local_id: "a", core_title: "a" }, | ||
| { source_local_id: "b", core_title: "b" }, | ||
| ], | ||
| localNodes: [node("a"), node("b")], | ||
| }); | ||
|
|
||
| expect(backfill.nodesToBackfill).toEqual([]); | ||
| expect(backfill.withCoreTitleCount).toBe(2); | ||
| expect(backfill.orphanedCount).toBe(0); | ||
| }); | ||
| }); | ||
|
|
||
| describe("mergeNodesBySourceLocalId", () => { | ||
| it("appends nodes that are not already in the batch", () => { | ||
| const merged = mergeNodesBySourceLocalId( | ||
| [node("a")], | ||
| [node("b"), node("c")], | ||
| ); | ||
|
|
||
| expect(merged.map((n) => n.source_local_id)).toEqual(["a", "b", "c"]); | ||
| }); | ||
|
|
||
| it("keeps the original node when both batches hold the same id", () => { | ||
| const original = node("a"); | ||
| const merged = mergeNodesBySourceLocalId([original], [node("a")]); | ||
|
|
||
| expect(merged).toEqual([original]); | ||
| }); | ||
| }); |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import { difference } from "@repo/utils/setOperations"; | ||
| import { | ||
| partitionByCoreTitle, | ||
| type CoreTitleProbeRow, | ||
| } from "@repo/database/lib/coreTitleBackfill"; | ||
| import { type RoamDiscourseNodeData } from "./getAllDiscourseNodesSince"; | ||
|
|
||
| export type CoreTitleBackfill = { | ||
| nodesToBackfill: RoamDiscourseNodeData[]; | ||
| withCoreTitleCount: number; | ||
| orphanedCount: number; | ||
| }; | ||
|
|
||
| export const buildCoreTitleBackfill = ({ | ||
| conceptRows, | ||
| localNodes, | ||
| }: { | ||
| conceptRows: CoreTitleProbeRow[]; | ||
| localNodes: RoamDiscourseNodeData[]; | ||
| }): CoreTitleBackfill => { | ||
| const { missingCoreTitleIds, withCoreTitleCount } = | ||
| partitionByCoreTitle(conceptRows); | ||
| const localIds = new Set(localNodes.map((node) => node.source_local_id)); | ||
| return { | ||
| nodesToBackfill: localNodes.filter((node) => | ||
| missingCoreTitleIds.has(node.source_local_id), | ||
| ), | ||
| withCoreTitleCount, | ||
| orphanedCount: difference(missingCoreTitleIds, localIds).size, | ||
| }; | ||
| }; | ||
|
|
||
| export const mergeNodesBySourceLocalId = ( | ||
| nodes: RoamDiscourseNodeData[], | ||
| additionalNodes: RoamDiscourseNodeData[], | ||
| ): RoamDiscourseNodeData[] => { | ||
| const nodesById = new Map(nodes.map((node) => [node.source_local_id, node])); | ||
| for (const node of additionalNodes) { | ||
| if (!nodesById.has(node.source_local_id)) { | ||
| nodesById.set(node.source_local_id, node); | ||
| } | ||
| } | ||
| return [...nodesById.values()]; | ||
| }; |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{content}capture instead of the first placeholderWhen an Obsidian node type uses an allowed format with another placeholder before
{content}(for example,{Source} - {content}),getDiscourseNodeFormatExpressioncreates one capture per placeholder butextractContentFromTitlealways returns capture 1. This therefore stores the source value ascore_title, corrupting the backfill and all subsequent concept upserts for that format; resolve the capture index for{content}as the Roam implementation does.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed on the defect; it is in #1318's producer (
apps/obsidian/src/utils/extractContentFromTitle.ts), which this PR's diff range includes but does not change. Added to the description's known-gaps list alongside the two other #1317/#1318 findings: all three store a non-null but wrongcore_title, and this migration treats any non-null value as already set, so those rows need a value-aware pass once the extractor is fixed.