Skip to content

ENG-2143 Add Roam and Obsidian round-trip tests - #1412

Open
sid597 wants to merge 4 commits into
eng-2143-source-adapters-basefrom
eng-2143-add-roam-and-obsidian-round-trip-tests
Open

ENG-2143 Add Roam and Obsidian round-trip tests#1412
sid597 wants to merge 4 commits into
eng-2143-source-adapters-basefrom
eng-2143-add-roam-and-obsidian-round-trip-tests

Conversation

@sid597

@sid597 sid597 commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator
eng-2143.mp4

Reviewer brief

Depends on #1380, #1381, #1382, and #1411 through eng-2143-source-adapters-base. Retarget to main after those land.

Both apps' adapters run in the tests. Only platform I/O and SQL storage are simulated.

Verification

  • Node 22: frozen install and pnpm ci:validate passed. 33 Obsidian tests and 269 Roam tests pass.
  • A Roam-only edit reruns the round-trip suite; an unchanged rerun uses the cache.

Loom video

74-second feature demo above.

Scope check

  • Ran $scope-check against ENG-2143 and the final diff.
  • Scope beyond Done When: None.

Local delegated full review

  • Full diff reviewed in a subagent with fresh context. No findings.

@vercel

vercel Bot commented Sep 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
discourse-graph Ready Ready Preview Sep 7, 2026 11:06am UTC

Request Review

@linear-code

linear-code Bot commented Sep 7, 2026

Copy link
Copy Markdown

ENG-2143

@supabase

supabase Bot commented Sep 7, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.


// Mirrors the storage boundary's local-reference resolution using fixed concept IDs.
// No adapter output is hand-written: the actual push result supplies the reference.
const storedSource = (input, references) => {

@sid597 sid597 Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helper simulates SQL reference resolution. Both apps' producers and importers run unchanged, so the assertions check the values passed between them.

@sid597
sid597 marked this pull request as ready for review September 7, 2026 10:45
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-07T10:51:12.635728Z ce3ee8a Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Devin Review

Comment thread apps/obsidian/src/utils/__tests__/sourceRoundTrip.test.mjs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ce3ee8ac2f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/obsidian/src/utils/__tests__/sourceRoundTrip.test.mjs
};

type Row = Record<string, unknown>;
export const createHarness = () => {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted from #1411 without behavior changes. All 20 importer test bodies remain identical to the stack base. The round-trip suite uses the same vault and database doubles.

@graphite-app

graphite-app Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

PR size/scope check

This PR is over our review-size guideline.

  • Recommended: ~200 lines changed
  • Acceptable limit: up to 400 lines when well-scoped/self-contained
  • Preferred file count: fewer than 5 files

Please split this into smaller PRs unless there is a clear reason the changes need to land together.

If keeping it as one PR, please add a brief justification covering:

  • What single problem this PR solves
  • Why the files/changes are coupled

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant