Skip to content

fix: re-land peek-to-thread handoff with callback pattern (#1464) - #1775

Merged
CraigBuckmaster merged 2 commits into
masterfrom
fix/1464-peek-handoff-callback-pattern
Apr 27, 2026
Merged

fix: re-land peek-to-thread handoff with callback pattern (#1464)#1775
CraigBuckmaster merged 2 commits into
masterfrom
fix/1464-peek-handoff-callback-pattern

Conversation

@CraigBuckmaster

Copy link
Copy Markdown
Owner

Closes#1464.

Re-lands the FAB peek → Amicus tab handoff that was originally shipped in PR #1500 and reverted in #1503 due to the iOS 26 TurboModule crash on TestFlight build (12). The underlying crash mechanism (ObjCTurboModule::performVoidMethodInvocation at RCTTurboModule.mm:441__cxa_rethrow → abort) was independently patched by app/patches/react-native+0.81.5.patch (already on master as part of the iOS 26 workaround triangle), so the specific (12) symptom cannot reoccur with the current build configuration.

What changed vs. the original PR

The original promotePeekToAmicusThread called navigation.getParent().navigate('AmicusTab', ...) from inside the service layer. That coupled a leaf component to the tab name and the navigator tree shape — the exact anti-pattern that motivated #1562 / #1563 (now subsumed into this card).

The re-land introduces a clean callback pattern:

  • Service returns a navigation-agnostic PromotePeekResult discriminated union: { kind: 'existing', threadId, ... } or { kind: 'new', promotedMessages, ... }. Zero React Navigation imports.
  • AmicusFab accepts an optional onContinueInTab(result) prop. When the parent provides it, the FAB delegates navigation entirely. When absent (e.g. a test mounting AmicusFab directly), the FAB falls back to internal navigation — defensive, keeps the component usable in isolation.
  • App.tsx provides the concrete navigation via navigationRef. This is now the single call site that knows about 'AmicusTab', 'Thread', and 'NewThread' screen names. If navigators are restructured, only this file changes.

File summary

FileChange
app/src/services/amicus/promotePeekToThread.tsNEW — navigation-agnostic service
app/src/services/amicus/__tests__/promotePeekToThread.test.tsNEW — 7 cases, no nav mocks
app/src/services/amicus/studyLaunch.tsRemoved deprecated promotePeekToAmicusThread + unused imports
app/src/services/amicus/__tests__/studyLaunch.test.tsDropped the two cases now covered in the new test file
app/src/services/amicus/index.tsExport rename
app/src/components/amicus/AmicusFab.tsxNew onContinueInTab? prop; internal nav as defensive fallback
app/App.tsxProvides handleAmicusContinueInTab via navigationRef

Thread-lookup logic (linked question → linked session → latest chapter thread) is preserved verbatim from the previous implementation. Only the navigation side-effect moves out of the service.

Verification

  • tsc --noEmit: clean (0 errors)
  • jest src/services/amicus/__tests__/promotePeekToThread.test.ts src/services/amicus/__tests__/studyLaunch.test.ts: 9/9 pass
  • Broader sweep jest src/services/amicus src/components/amicus: 235/236 pass; the one failure is a pre-existing flaky afterEach timeout in AmicusPeekSheet.test.tsx that passes cleanly when run in isolation (5/5)
  • eslint: 0 errors on changed files

Rollback plan

If this re-introduces a crash on TestFlight, the rollback is mechanical:

  1. git revert <merge-commit> on master
  2. The reverted state restores promotePeekToAmicusThread in studyLaunch.ts, the AmicusFab original handleContinueInTab, and removes the new files
  3. No data migrations or persisted-state changes are involved — this is a pure refactor of the in-process control flow

The FAB's defensive internal-navigation fallback also means that if App.tsx callback wiring is ever lost (e.g. a stray refactor strips the prop), the feature still works — it just isn't architecturally clean.

What still belongs to the epic

With this merged and verified on TestFlight, all 27 child issues of Epic #1446 are complete and the epic can be closed.

Replaces `promotePeekToAmicusThread` (which called `navigation.getParent()`
from the service layer) with a navigation-agnostic `promotePeekToThread`
that returns a discriminated `PromotePeekResult` describing what should
happen next. The actual `navigate(...)` call moves to App.tsx via
`navigationRef`, so the navigation tree shape lives in one place.
This is the re-land of PR #1500 (reverted in #1503 due to the iOS 26
TurboModule crash). The underlying crash mechanism is fixed by
`app/patches/react-native+0.81.5.patch` already in master, so the
specific (12) symptom cannot reoccur.
Architectural improvements vs. the original PR:
- Service has zero React Navigation imports
- Tests don't mock useNavigation/getParent
- Restructuring navigators touches one call site (App.tsx), not service code
- Subsumes the architectural concerns from #1562 / #1563 (closed)
Files:
- NEW app/src/services/amicus/promotePeekToThread.ts
- NEW app/src/services/amicus/__tests__/promotePeekToThread.test.ts
- MOD app/src/services/amicus/studyLaunch.ts (removed deprecated function)
- MOD app/src/services/amicus/__tests__/studyLaunch.test.ts (drop the two cases now in the new file)
- MOD app/src/services/amicus/index.ts (export rename)
- MOD app/src/components/amicus/AmicusFab.tsx (accept onContinueInTab prop, internal nav as defensive fallback)
- MOD app/App.tsx (provide onContinueInTab via navigationRef)
Verification:
- tsc --noEmit: clean
- jest src/services/amicus: 9/9 pass on the changed surface
- eslint: 0 errors on changed files
@github-actions

github-actionsBot commented Apr 27, 2026

Copy link
Copy Markdown

Test Results

✅ All tests passed

PassedFailedTotal
Tests✅ 3962❌ 03962
Suites✅ 529❌ 0529

Coverage

StatementsBranchesFunctionsLines

⏱️ Duration: 94.4s

CI's `expo-doctor — dep version check` step started failing on this
branch (and would fail on master too) after expo-doctor's bundled
'expected versions' map advanced to a newer SDK 54 patch level.
Nine deps were one patch behind:
expo-asset: ~12.0.12 → ~12.0.13
expo-auth-session: ~7.0.10 → ~7.0.11
expo-crypto: ~15.0.8 → ~15.0.9
expo-dev-client: ~6.0.20 → ~6.0.21
expo-file-system: ~19.0.21 → ~19.0.22
expo-notifications: ~0.32.16 → ~0.32.17
expo-screen-orientation: ~9.0.8 → ~9.0.9
expo-updates: ~29.0.16 → ~29.0.17
expo-web-browser: ~15.0.10 → ~15.0.11
`expo` itself was already at ~54.0.34 in package.json; only the
lockfile + transitive deps needed a refresh.
Verification:
- npx expo-doctor@latest: 17/17 checks passed
- tsc --noEmit: clean
- jest src/services/amicus (changed surface): 9/9 pass
- patch-package re-applied react-native+0.81.5.patch cleanly
@CraigBuckmaster
CraigBuckmaster merged commit 51f9e7d into masterApr 27, 2026
6 checks passed
@CraigBuckmaster
CraigBuckmaster deleted the fix/1464-peek-handoff-callback-pattern branch April 27, 2026 20:27
Sign up for freeto 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.

ai-partner: continue-in-tab handoff from peek (re-land with callback pattern)

1 participant

@CraigBuckmaster
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
fix: re-land peek-to-thread handoff with callback pattern (#1464) by CraigBuckmaster · Pull Request #1775 · CraigBuckmaster/ScriptureDeepDive · GitHub
Skip to content

fix: re-land peek-to-thread handoff with callback pattern (#1464) - #1775

Merged
CraigBuckmaster merged 2 commits into
masterfrom
fix/1464-peek-handoff-callback-pattern
Apr 27, 2026
Merged

fix: re-land peek-to-thread handoff with callback pattern (#1464)#1775
CraigBuckmaster merged 2 commits into
masterfrom
fix/1464-peek-handoff-callback-pattern

Conversation

@CraigBuckmaster

Copy link
Copy Markdown
Owner

Closes#1464.

Re-lands the FAB peek → Amicus tab handoff that was originally shipped in PR #1500 and reverted in #1503 due to the iOS 26 TurboModule crash on TestFlight build (12). The underlying crash mechanism (ObjCTurboModule::performVoidMethodInvocation at RCTTurboModule.mm:441__cxa_rethrow → abort) was independently patched by app/patches/react-native+0.81.5.patch (already on master as part of the iOS 26 workaround triangle), so the specific (12) symptom cannot reoccur with the current build configuration.

What changed vs. the original PR

The original promotePeekToAmicusThread called navigation.getParent().navigate('AmicusTab', ...) from inside the service layer. That coupled a leaf component to the tab name and the navigator tree shape — the exact anti-pattern that motivated #1562 / #1563 (now subsumed into this card).

The re-land introduces a clean callback pattern:

  • Service returns a navigation-agnostic PromotePeekResult discriminated union: { kind: 'existing', threadId, ... } or { kind: 'new', promotedMessages, ... }. Zero React Navigation imports.
  • AmicusFab accepts an optional onContinueInTab(result) prop. When the parent provides it, the FAB delegates navigation entirely. When absent (e.g. a test mounting AmicusFab directly), the FAB falls back to internal navigation — defensive, keeps the component usable in isolation.
  • App.tsx provides the concrete navigation via navigationRef. This is now the single call site that knows about 'AmicusTab', 'Thread', and 'NewThread' screen names. If navigators are restructured, only this file changes.

File summary

FileChange
app/src/services/amicus/promotePeekToThread.tsNEW — navigation-agnostic service
app/src/services/amicus/__tests__/promotePeekToThread.test.tsNEW — 7 cases, no nav mocks
app/src/services/amicus/studyLaunch.tsRemoved deprecated promotePeekToAmicusThread + unused imports
app/src/services/amicus/__tests__/studyLaunch.test.tsDropped the two cases now covered in the new test file
app/src/services/amicus/index.tsExport rename
app/src/components/amicus/AmicusFab.tsxNew onContinueInTab? prop; internal nav as defensive fallback
app/App.tsxProvides handleAmicusContinueInTab via navigationRef

Thread-lookup logic (linked question → linked session → latest chapter thread) is preserved verbatim from the previous implementation. Only the navigation side-effect moves out of the service.

Verification

  • tsc --noEmit: clean (0 errors)
  • jest src/services/amicus/__tests__/promotePeekToThread.test.ts src/services/amicus/__tests__/studyLaunch.test.ts: 9/9 pass
  • Broader sweep jest src/services/amicus src/components/amicus: 235/236 pass; the one failure is a pre-existing flaky afterEach timeout in AmicusPeekSheet.test.tsx that passes cleanly when run in isolation (5/5)
  • eslint: 0 errors on changed files

Rollback plan

If this re-introduces a crash on TestFlight, the rollback is mechanical:

  1. git revert <merge-commit> on master
  2. The reverted state restores promotePeekToAmicusThread in studyLaunch.ts, the AmicusFab original handleContinueInTab, and removes the new files
  3. No data migrations or persisted-state changes are involved — this is a pure refactor of the in-process control flow

The FAB's defensive internal-navigation fallback also means that if App.tsx callback wiring is ever lost (e.g. a stray refactor strips the prop), the feature still works — it just isn't architecturally clean.

What still belongs to the epic

With this merged and verified on TestFlight, all 27 child issues of Epic #1446 are complete and the epic can be closed.

Replaces `promotePeekToAmicusThread` (which called `navigation.getParent()`
from the service layer) with a navigation-agnostic `promotePeekToThread`
that returns a discriminated `PromotePeekResult` describing what should
happen next. The actual `navigate(...)` call moves to App.tsx via
`navigationRef`, so the navigation tree shape lives in one place.
This is the re-land of PR #1500 (reverted in #1503 due to the iOS 26
TurboModule crash). The underlying crash mechanism is fixed by
`app/patches/react-native+0.81.5.patch` already in master, so the
specific (12) symptom cannot reoccur.
Architectural improvements vs. the original PR:
- Service has zero React Navigation imports
- Tests don't mock useNavigation/getParent
- Restructuring navigators touches one call site (App.tsx), not service code
- Subsumes the architectural concerns from #1562 / #1563 (closed)
Files:
- NEW app/src/services/amicus/promotePeekToThread.ts
- NEW app/src/services/amicus/__tests__/promotePeekToThread.test.ts
- MOD app/src/services/amicus/studyLaunch.ts (removed deprecated function)
- MOD app/src/services/amicus/__tests__/studyLaunch.test.ts (drop the two cases now in the new file)
- MOD app/src/services/amicus/index.ts (export rename)
- MOD app/src/components/amicus/AmicusFab.tsx (accept onContinueInTab prop, internal nav as defensive fallback)
- MOD app/App.tsx (provide onContinueInTab via navigationRef)
Verification:
- tsc --noEmit: clean
- jest src/services/amicus: 9/9 pass on the changed surface
- eslint: 0 errors on changed files
@github-actions

github-actionsBot commented Apr 27, 2026

Copy link
Copy Markdown

Test Results

✅ All tests passed

PassedFailedTotal
Tests✅ 3962❌ 03962
Suites✅ 529❌ 0529

Coverage

StatementsBranchesFunctionsLines

⏱️ Duration: 94.4s

CI's `expo-doctor — dep version check` step started failing on this
branch (and would fail on master too) after expo-doctor's bundled
'expected versions' map advanced to a newer SDK 54 patch level.
Nine deps were one patch behind:
expo-asset: ~12.0.12 → ~12.0.13
expo-auth-session: ~7.0.10 → ~7.0.11
expo-crypto: ~15.0.8 → ~15.0.9
expo-dev-client: ~6.0.20 → ~6.0.21
expo-file-system: ~19.0.21 → ~19.0.22
expo-notifications: ~0.32.16 → ~0.32.17
expo-screen-orientation: ~9.0.8 → ~9.0.9
expo-updates: ~29.0.16 → ~29.0.17
expo-web-browser: ~15.0.10 → ~15.0.11
`expo` itself was already at ~54.0.34 in package.json; only the
lockfile + transitive deps needed a refresh.
Verification:
- npx expo-doctor@latest: 17/17 checks passed
- tsc --noEmit: clean
- jest src/services/amicus (changed surface): 9/9 pass
- patch-package re-applied react-native+0.81.5.patch cleanly
@CraigBuckmaster
CraigBuckmaster merged commit 51f9e7d into masterApr 27, 2026
6 checks passed
@CraigBuckmaster
CraigBuckmaster deleted the fix/1464-peek-handoff-callback-pattern branch April 27, 2026 20:27
Sign up for freeto 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.

ai-partner: continue-in-tab handoff from peek (re-land with callback pattern)

1 participant

@CraigBuckmaster
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix: re-land peek-to-thread handoff with callback pattern (#1464) by CraigBuckmaster · Pull Request #1775 · CraigBuckmaster/ScriptureDeepDive · GitHub
Skip to content

fix: re-land peek-to-thread handoff with callback pattern (#1464) - #1775

Merged
CraigBuckmaster merged 2 commits into
masterfrom
fix/1464-peek-handoff-callback-pattern
Apr 27, 2026
Merged

fix: re-land peek-to-thread handoff with callback pattern (#1464)#1775
CraigBuckmaster merged 2 commits into
masterfrom
fix/1464-peek-handoff-callback-pattern

Conversation

@CraigBuckmaster

Copy link
Copy Markdown
Owner

Closes#1464.

Re-lands the FAB peek → Amicus tab handoff that was originally shipped in PR #1500 and reverted in #1503 due to the iOS 26 TurboModule crash on TestFlight build (12). The underlying crash mechanism (ObjCTurboModule::performVoidMethodInvocation at RCTTurboModule.mm:441__cxa_rethrow → abort) was independently patched by app/patches/react-native+0.81.5.patch (already on master as part of the iOS 26 workaround triangle), so the specific (12) symptom cannot reoccur with the current build configuration.

What changed vs. the original PR

The original promotePeekToAmicusThread called navigation.getParent().navigate('AmicusTab', ...) from inside the service layer. That coupled a leaf component to the tab name and the navigator tree shape — the exact anti-pattern that motivated #1562 / #1563 (now subsumed into this card).

The re-land introduces a clean callback pattern:

  • Service returns a navigation-agnostic PromotePeekResult discriminated union: { kind: 'existing', threadId, ... } or { kind: 'new', promotedMessages, ... }. Zero React Navigation imports.
  • AmicusFab accepts an optional onContinueInTab(result) prop. When the parent provides it, the FAB delegates navigation entirely. When absent (e.g. a test mounting AmicusFab directly), the FAB falls back to internal navigation — defensive, keeps the component usable in isolation.
  • App.tsx provides the concrete navigation via navigationRef. This is now the single call site that knows about 'AmicusTab', 'Thread', and 'NewThread' screen names. If navigators are restructured, only this file changes.

File summary

FileChange
app/src/services/amicus/promotePeekToThread.tsNEW — navigation-agnostic service
app/src/services/amicus/__tests__/promotePeekToThread.test.tsNEW — 7 cases, no nav mocks
app/src/services/amicus/studyLaunch.tsRemoved deprecated promotePeekToAmicusThread + unused imports
app/src/services/amicus/__tests__/studyLaunch.test.tsDropped the two cases now covered in the new test file
app/src/services/amicus/index.tsExport rename
app/src/components/amicus/AmicusFab.tsxNew onContinueInTab? prop; internal nav as defensive fallback
app/App.tsxProvides handleAmicusContinueInTab via navigationRef

Thread-lookup logic (linked question → linked session → latest chapter thread) is preserved verbatim from the previous implementation. Only the navigation side-effect moves out of the service.

Verification

  • tsc --noEmit: clean (0 errors)
  • jest src/services/amicus/__tests__/promotePeekToThread.test.ts src/services/amicus/__tests__/studyLaunch.test.ts: 9/9 pass
  • Broader sweep jest src/services/amicus src/components/amicus: 235/236 pass; the one failure is a pre-existing flaky afterEach timeout in AmicusPeekSheet.test.tsx that passes cleanly when run in isolation (5/5)
  • eslint: 0 errors on changed files

Rollback plan

If this re-introduces a crash on TestFlight, the rollback is mechanical:

  1. git revert <merge-commit> on master
  2. The reverted state restores promotePeekToAmicusThread in studyLaunch.ts, the AmicusFab original handleContinueInTab, and removes the new files
  3. No data migrations or persisted-state changes are involved — this is a pure refactor of the in-process control flow

The FAB's defensive internal-navigation fallback also means that if App.tsx callback wiring is ever lost (e.g. a stray refactor strips the prop), the feature still works — it just isn't architecturally clean.

What still belongs to the epic

With this merged and verified on TestFlight, all 27 child issues of Epic #1446 are complete and the epic can be closed.

Replaces `promotePeekToAmicusThread` (which called `navigation.getParent()`
from the service layer) with a navigation-agnostic `promotePeekToThread`
that returns a discriminated `PromotePeekResult` describing what should
happen next. The actual `navigate(...)` call moves to App.tsx via
`navigationRef`, so the navigation tree shape lives in one place.
This is the re-land of PR #1500 (reverted in #1503 due to the iOS 26
TurboModule crash). The underlying crash mechanism is fixed by
`app/patches/react-native+0.81.5.patch` already in master, so the
specific (12) symptom cannot reoccur.
Architectural improvements vs. the original PR:
- Service has zero React Navigation imports
- Tests don't mock useNavigation/getParent
- Restructuring navigators touches one call site (App.tsx), not service code
- Subsumes the architectural concerns from #1562 / #1563 (closed)
Files:
- NEW app/src/services/amicus/promotePeekToThread.ts
- NEW app/src/services/amicus/__tests__/promotePeekToThread.test.ts
- MOD app/src/services/amicus/studyLaunch.ts (removed deprecated function)
- MOD app/src/services/amicus/__tests__/studyLaunch.test.ts (drop the two cases now in the new file)
- MOD app/src/services/amicus/index.ts (export rename)
- MOD app/src/components/amicus/AmicusFab.tsx (accept onContinueInTab prop, internal nav as defensive fallback)
- MOD app/App.tsx (provide onContinueInTab via navigationRef)
Verification:
- tsc --noEmit: clean
- jest src/services/amicus: 9/9 pass on the changed surface
- eslint: 0 errors on changed files
@github-actions

github-actionsBot commented Apr 27, 2026

Copy link
Copy Markdown

Test Results

✅ All tests passed

PassedFailedTotal
Tests✅ 3962❌ 03962
Suites✅ 529❌ 0529

Coverage

StatementsBranchesFunctionsLines

⏱️ Duration: 94.4s

CI's `expo-doctor — dep version check` step started failing on this
branch (and would fail on master too) after expo-doctor's bundled
'expected versions' map advanced to a newer SDK 54 patch level.
Nine deps were one patch behind:
expo-asset: ~12.0.12 → ~12.0.13
expo-auth-session: ~7.0.10 → ~7.0.11
expo-crypto: ~15.0.8 → ~15.0.9
expo-dev-client: ~6.0.20 → ~6.0.21
expo-file-system: ~19.0.21 → ~19.0.22
expo-notifications: ~0.32.16 → ~0.32.17
expo-screen-orientation: ~9.0.8 → ~9.0.9
expo-updates: ~29.0.16 → ~29.0.17
expo-web-browser: ~15.0.10 → ~15.0.11
`expo` itself was already at ~54.0.34 in package.json; only the
lockfile + transitive deps needed a refresh.
Verification:
- npx expo-doctor@latest: 17/17 checks passed
- tsc --noEmit: clean
- jest src/services/amicus (changed surface): 9/9 pass
- patch-package re-applied react-native+0.81.5.patch cleanly
@CraigBuckmaster
CraigBuckmaster merged commit 51f9e7d into masterApr 27, 2026
6 checks passed
@CraigBuckmaster
CraigBuckmaster deleted the fix/1464-peek-handoff-callback-pattern branch April 27, 2026 20:27
Sign up for freeto 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.

ai-partner: continue-in-tab handoff from peek (re-land with callback pattern)

1 participant

@CraigBuckmaster
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix: re-land peek-to-thread handoff with callback pattern (#1464) by CraigBuckmaster · Pull Request #1775 · CraigBuckmaster/ScriptureDeepDive · GitHub
Skip to content

fix: re-land peek-to-thread handoff with callback pattern (#1464) - #1775

Merged
CraigBuckmaster merged 2 commits into
masterfrom
fix/1464-peek-handoff-callback-pattern
Apr 27, 2026
Merged

fix: re-land peek-to-thread handoff with callback pattern (#1464)#1775
CraigBuckmaster merged 2 commits into
masterfrom
fix/1464-peek-handoff-callback-pattern

Conversation

@CraigBuckmaster

Copy link
Copy Markdown
Owner

Closes#1464.

Re-lands the FAB peek → Amicus tab handoff that was originally shipped in PR #1500 and reverted in #1503 due to the iOS 26 TurboModule crash on TestFlight build (12). The underlying crash mechanism (ObjCTurboModule::performVoidMethodInvocation at RCTTurboModule.mm:441__cxa_rethrow → abort) was independently patched by app/patches/react-native+0.81.5.patch (already on master as part of the iOS 26 workaround triangle), so the specific (12) symptom cannot reoccur with the current build configuration.

What changed vs. the original PR

The original promotePeekToAmicusThread called navigation.getParent().navigate('AmicusTab', ...) from inside the service layer. That coupled a leaf component to the tab name and the navigator tree shape — the exact anti-pattern that motivated #1562 / #1563 (now subsumed into this card).

The re-land introduces a clean callback pattern:

  • Service returns a navigation-agnostic PromotePeekResult discriminated union: { kind: 'existing', threadId, ... } or { kind: 'new', promotedMessages, ... }. Zero React Navigation imports.
  • AmicusFab accepts an optional onContinueInTab(result) prop. When the parent provides it, the FAB delegates navigation entirely. When absent (e.g. a test mounting AmicusFab directly), the FAB falls back to internal navigation — defensive, keeps the component usable in isolation.
  • App.tsx provides the concrete navigation via navigationRef. This is now the single call site that knows about 'AmicusTab', 'Thread', and 'NewThread' screen names. If navigators are restructured, only this file changes.

File summary

FileChange
app/src/services/amicus/promotePeekToThread.tsNEW — navigation-agnostic service
app/src/services/amicus/__tests__/promotePeekToThread.test.tsNEW — 7 cases, no nav mocks
app/src/services/amicus/studyLaunch.tsRemoved deprecated promotePeekToAmicusThread + unused imports
app/src/services/amicus/__tests__/studyLaunch.test.tsDropped the two cases now covered in the new test file
app/src/services/amicus/index.tsExport rename
app/src/components/amicus/AmicusFab.tsxNew onContinueInTab? prop; internal nav as defensive fallback
app/App.tsxProvides handleAmicusContinueInTab via navigationRef

Thread-lookup logic (linked question → linked session → latest chapter thread) is preserved verbatim from the previous implementation. Only the navigation side-effect moves out of the service.

Verification

  • tsc --noEmit: clean (0 errors)
  • jest src/services/amicus/__tests__/promotePeekToThread.test.ts src/services/amicus/__tests__/studyLaunch.test.ts: 9/9 pass
  • Broader sweep jest src/services/amicus src/components/amicus: 235/236 pass; the one failure is a pre-existing flaky afterEach timeout in AmicusPeekSheet.test.tsx that passes cleanly when run in isolation (5/5)
  • eslint: 0 errors on changed files

Rollback plan

If this re-introduces a crash on TestFlight, the rollback is mechanical:

  1. git revert <merge-commit> on master
  2. The reverted state restores promotePeekToAmicusThread in studyLaunch.ts, the AmicusFab original handleContinueInTab, and removes the new files
  3. No data migrations or persisted-state changes are involved — this is a pure refactor of the in-process control flow

The FAB's defensive internal-navigation fallback also means that if App.tsx callback wiring is ever lost (e.g. a stray refactor strips the prop), the feature still works — it just isn't architecturally clean.

What still belongs to the epic

With this merged and verified on TestFlight, all 27 child issues of Epic #1446 are complete and the epic can be closed.

Replaces `promotePeekToAmicusThread` (which called `navigation.getParent()`
from the service layer) with a navigation-agnostic `promotePeekToThread`
that returns a discriminated `PromotePeekResult` describing what should
happen next. The actual `navigate(...)` call moves to App.tsx via
`navigationRef`, so the navigation tree shape lives in one place.
This is the re-land of PR #1500 (reverted in #1503 due to the iOS 26
TurboModule crash). The underlying crash mechanism is fixed by
`app/patches/react-native+0.81.5.patch` already in master, so the
specific (12) symptom cannot reoccur.
Architectural improvements vs. the original PR:
- Service has zero React Navigation imports
- Tests don't mock useNavigation/getParent
- Restructuring navigators touches one call site (App.tsx), not service code
- Subsumes the architectural concerns from #1562 / #1563 (closed)
Files:
- NEW app/src/services/amicus/promotePeekToThread.ts
- NEW app/src/services/amicus/__tests__/promotePeekToThread.test.ts
- MOD app/src/services/amicus/studyLaunch.ts (removed deprecated function)
- MOD app/src/services/amicus/__tests__/studyLaunch.test.ts (drop the two cases now in the new file)
- MOD app/src/services/amicus/index.ts (export rename)
- MOD app/src/components/amicus/AmicusFab.tsx (accept onContinueInTab prop, internal nav as defensive fallback)
- MOD app/App.tsx (provide onContinueInTab via navigationRef)
Verification:
- tsc --noEmit: clean
- jest src/services/amicus: 9/9 pass on the changed surface
- eslint: 0 errors on changed files
@github-actions

github-actionsBot commented Apr 27, 2026

Copy link
Copy Markdown

Test Results

✅ All tests passed

PassedFailedTotal
Tests✅ 3962❌ 03962
Suites✅ 529❌ 0529

Coverage

StatementsBranchesFunctionsLines

⏱️ Duration: 94.4s

CI's `expo-doctor — dep version check` step started failing on this
branch (and would fail on master too) after expo-doctor's bundled
'expected versions' map advanced to a newer SDK 54 patch level.
Nine deps were one patch behind:
expo-asset: ~12.0.12 → ~12.0.13
expo-auth-session: ~7.0.10 → ~7.0.11
expo-crypto: ~15.0.8 → ~15.0.9
expo-dev-client: ~6.0.20 → ~6.0.21
expo-file-system: ~19.0.21 → ~19.0.22
expo-notifications: ~0.32.16 → ~0.32.17
expo-screen-orientation: ~9.0.8 → ~9.0.9
expo-updates: ~29.0.16 → ~29.0.17
expo-web-browser: ~15.0.10 → ~15.0.11
`expo` itself was already at ~54.0.34 in package.json; only the
lockfile + transitive deps needed a refresh.
Verification:
- npx expo-doctor@latest: 17/17 checks passed
- tsc --noEmit: clean
- jest src/services/amicus (changed surface): 9/9 pass
- patch-package re-applied react-native+0.81.5.patch cleanly
@CraigBuckmaster
CraigBuckmaster merged commit 51f9e7d into masterApr 27, 2026
6 checks passed
@CraigBuckmaster
CraigBuckmaster deleted the fix/1464-peek-handoff-callback-pattern branch April 27, 2026 20:27
Sign up for freeto 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.

ai-partner: continue-in-tab handoff from peek (re-land with callback pattern)

1 participant

@CraigBuckmaster
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' fix: re-land peek-to-thread handoff with callback pattern (#1464) by CraigBuckmaster · Pull Request #1775 · CraigBuckmaster/ScriptureDeepDive · GitHub
Skip to content

fix: re-land peek-to-thread handoff with callback pattern (#1464) - #1775

Merged
CraigBuckmaster merged 2 commits into
masterfrom
fix/1464-peek-handoff-callback-pattern
Apr 27, 2026
Merged

fix: re-land peek-to-thread handoff with callback pattern (#1464)#1775
CraigBuckmaster merged 2 commits into
masterfrom
fix/1464-peek-handoff-callback-pattern

Conversation

@CraigBuckmaster

Copy link
Copy Markdown
Owner

Closes#1464.

Re-lands the FAB peek → Amicus tab handoff that was originally shipped in PR #1500 and reverted in #1503 due to the iOS 26 TurboModule crash on TestFlight build (12). The underlying crash mechanism (ObjCTurboModule::performVoidMethodInvocation at RCTTurboModule.mm:441__cxa_rethrow → abort) was independently patched by app/patches/react-native+0.81.5.patch (already on master as part of the iOS 26 workaround triangle), so the specific (12) symptom cannot reoccur with the current build configuration.

What changed vs. the original PR

The original promotePeekToAmicusThread called navigation.getParent().navigate('AmicusTab', ...) from inside the service layer. That coupled a leaf component to the tab name and the navigator tree shape — the exact anti-pattern that motivated #1562 / #1563 (now subsumed into this card).

The re-land introduces a clean callback pattern:

  • Service returns a navigation-agnostic PromotePeekResult discriminated union: { kind: 'existing', threadId, ... } or { kind: 'new', promotedMessages, ... }. Zero React Navigation imports.
  • AmicusFab accepts an optional onContinueInTab(result) prop. When the parent provides it, the FAB delegates navigation entirely. When absent (e.g. a test mounting AmicusFab directly), the FAB falls back to internal navigation — defensive, keeps the component usable in isolation.
  • App.tsx provides the concrete navigation via navigationRef. This is now the single call site that knows about 'AmicusTab', 'Thread', and 'NewThread' screen names. If navigators are restructured, only this file changes.

File summary

FileChange
app/src/services/amicus/promotePeekToThread.tsNEW — navigation-agnostic service
app/src/services/amicus/__tests__/promotePeekToThread.test.tsNEW — 7 cases, no nav mocks
app/src/services/amicus/studyLaunch.tsRemoved deprecated promotePeekToAmicusThread + unused imports
app/src/services/amicus/__tests__/studyLaunch.test.tsDropped the two cases now covered in the new test file
app/src/services/amicus/index.tsExport rename
app/src/components/amicus/AmicusFab.tsxNew onContinueInTab? prop; internal nav as defensive fallback
app/App.tsxProvides handleAmicusContinueInTab via navigationRef

Thread-lookup logic (linked question → linked session → latest chapter thread) is preserved verbatim from the previous implementation. Only the navigation side-effect moves out of the service.

Verification

  • tsc --noEmit: clean (0 errors)
  • jest src/services/amicus/__tests__/promotePeekToThread.test.ts src/services/amicus/__tests__/studyLaunch.test.ts: 9/9 pass
  • Broader sweep jest src/services/amicus src/components/amicus: 235/236 pass; the one failure is a pre-existing flaky afterEach timeout in AmicusPeekSheet.test.tsx that passes cleanly when run in isolation (5/5)
  • eslint: 0 errors on changed files

Rollback plan

If this re-introduces a crash on TestFlight, the rollback is mechanical:

  1. git revert <merge-commit> on master
  2. The reverted state restores promotePeekToAmicusThread in studyLaunch.ts, the AmicusFab original handleContinueInTab, and removes the new files
  3. No data migrations or persisted-state changes are involved — this is a pure refactor of the in-process control flow

The FAB's defensive internal-navigation fallback also means that if App.tsx callback wiring is ever lost (e.g. a stray refactor strips the prop), the feature still works — it just isn't architecturally clean.

What still belongs to the epic

With this merged and verified on TestFlight, all 27 child issues of Epic #1446 are complete and the epic can be closed.

Replaces `promotePeekToAmicusThread` (which called `navigation.getParent()`
from the service layer) with a navigation-agnostic `promotePeekToThread`
that returns a discriminated `PromotePeekResult` describing what should
happen next. The actual `navigate(...)` call moves to App.tsx via
`navigationRef`, so the navigation tree shape lives in one place.
This is the re-land of PR #1500 (reverted in #1503 due to the iOS 26
TurboModule crash). The underlying crash mechanism is fixed by
`app/patches/react-native+0.81.5.patch` already in master, so the
specific (12) symptom cannot reoccur.
Architectural improvements vs. the original PR:
- Service has zero React Navigation imports
- Tests don't mock useNavigation/getParent
- Restructuring navigators touches one call site (App.tsx), not service code
- Subsumes the architectural concerns from #1562 / #1563 (closed)
Files:
- NEW app/src/services/amicus/promotePeekToThread.ts
- NEW app/src/services/amicus/__tests__/promotePeekToThread.test.ts
- MOD app/src/services/amicus/studyLaunch.ts (removed deprecated function)
- MOD app/src/services/amicus/__tests__/studyLaunch.test.ts (drop the two cases now in the new file)
- MOD app/src/services/amicus/index.ts (export rename)
- MOD app/src/components/amicus/AmicusFab.tsx (accept onContinueInTab prop, internal nav as defensive fallback)
- MOD app/App.tsx (provide onContinueInTab via navigationRef)
Verification:
- tsc --noEmit: clean
- jest src/services/amicus: 9/9 pass on the changed surface
- eslint: 0 errors on changed files
@github-actions

github-actionsBot commented Apr 27, 2026

Copy link
Copy Markdown

Test Results

✅ All tests passed

PassedFailedTotal
Tests✅ 3962❌ 03962
Suites✅ 529❌ 0529

Coverage

StatementsBranchesFunctionsLines

⏱️ Duration: 94.4s

CI's `expo-doctor — dep version check` step started failing on this
branch (and would fail on master too) after expo-doctor's bundled
'expected versions' map advanced to a newer SDK 54 patch level.
Nine deps were one patch behind:
expo-asset: ~12.0.12 → ~12.0.13
expo-auth-session: ~7.0.10 → ~7.0.11
expo-crypto: ~15.0.8 → ~15.0.9
expo-dev-client: ~6.0.20 → ~6.0.21
expo-file-system: ~19.0.21 → ~19.0.22
expo-notifications: ~0.32.16 → ~0.32.17
expo-screen-orientation: ~9.0.8 → ~9.0.9
expo-updates: ~29.0.16 → ~29.0.17
expo-web-browser: ~15.0.10 → ~15.0.11
`expo` itself was already at ~54.0.34 in package.json; only the
lockfile + transitive deps needed a refresh.
Verification:
- npx expo-doctor@latest: 17/17 checks passed
- tsc --noEmit: clean
- jest src/services/amicus (changed surface): 9/9 pass
- patch-package re-applied react-native+0.81.5.patch cleanly
@CraigBuckmaster
CraigBuckmaster merged commit 51f9e7d into masterApr 27, 2026
6 checks passed
@CraigBuckmaster
CraigBuckmaster deleted the fix/1464-peek-handoff-callback-pattern branch April 27, 2026 20:27
Sign up for freeto 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.

ai-partner: continue-in-tab handoff from peek (re-land with callback pattern)

1 participant

@CraigBuckmaster
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix: re-land peek-to-thread handoff with callback pattern (#1464) by CraigBuckmaster · Pull Request #1775 · CraigBuckmaster/ScriptureDeepDive · GitHub
Skip to content

fix: re-land peek-to-thread handoff with callback pattern (#1464) - #1775

Merged
CraigBuckmaster merged 2 commits into
masterfrom
fix/1464-peek-handoff-callback-pattern
Apr 27, 2026
Merged

fix: re-land peek-to-thread handoff with callback pattern (#1464)#1775
CraigBuckmaster merged 2 commits into
masterfrom
fix/1464-peek-handoff-callback-pattern

Conversation

@CraigBuckmaster

Copy link
Copy Markdown
Owner

Closes#1464.

Re-lands the FAB peek → Amicus tab handoff that was originally shipped in PR #1500 and reverted in #1503 due to the iOS 26 TurboModule crash on TestFlight build (12). The underlying crash mechanism (ObjCTurboModule::performVoidMethodInvocation at RCTTurboModule.mm:441__cxa_rethrow → abort) was independently patched by app/patches/react-native+0.81.5.patch (already on master as part of the iOS 26 workaround triangle), so the specific (12) symptom cannot reoccur with the current build configuration.

What changed vs. the original PR

The original promotePeekToAmicusThread called navigation.getParent().navigate('AmicusTab', ...) from inside the service layer. That coupled a leaf component to the tab name and the navigator tree shape — the exact anti-pattern that motivated #1562 / #1563 (now subsumed into this card).

The re-land introduces a clean callback pattern:

  • Service returns a navigation-agnostic PromotePeekResult discriminated union: { kind: 'existing', threadId, ... } or { kind: 'new', promotedMessages, ... }. Zero React Navigation imports.
  • AmicusFab accepts an optional onContinueInTab(result) prop. When the parent provides it, the FAB delegates navigation entirely. When absent (e.g. a test mounting AmicusFab directly), the FAB falls back to internal navigation — defensive, keeps the component usable in isolation.
  • App.tsx provides the concrete navigation via navigationRef. This is now the single call site that knows about 'AmicusTab', 'Thread', and 'NewThread' screen names. If navigators are restructured, only this file changes.

File summary

FileChange
app/src/services/amicus/promotePeekToThread.tsNEW — navigation-agnostic service
app/src/services/amicus/__tests__/promotePeekToThread.test.tsNEW — 7 cases, no nav mocks
app/src/services/amicus/studyLaunch.tsRemoved deprecated promotePeekToAmicusThread + unused imports
app/src/services/amicus/__tests__/studyLaunch.test.tsDropped the two cases now covered in the new test file
app/src/services/amicus/index.tsExport rename
app/src/components/amicus/AmicusFab.tsxNew onContinueInTab? prop; internal nav as defensive fallback
app/App.tsxProvides handleAmicusContinueInTab via navigationRef

Thread-lookup logic (linked question → linked session → latest chapter thread) is preserved verbatim from the previous implementation. Only the navigation side-effect moves out of the service.

Verification

  • tsc --noEmit: clean (0 errors)
  • jest src/services/amicus/__tests__/promotePeekToThread.test.ts src/services/amicus/__tests__/studyLaunch.test.ts: 9/9 pass
  • Broader sweep jest src/services/amicus src/components/amicus: 235/236 pass; the one failure is a pre-existing flaky afterEach timeout in AmicusPeekSheet.test.tsx that passes cleanly when run in isolation (5/5)
  • eslint: 0 errors on changed files

Rollback plan

If this re-introduces a crash on TestFlight, the rollback is mechanical:

  1. git revert <merge-commit> on master
  2. The reverted state restores promotePeekToAmicusThread in studyLaunch.ts, the AmicusFab original handleContinueInTab, and removes the new files
  3. No data migrations or persisted-state changes are involved — this is a pure refactor of the in-process control flow

The FAB's defensive internal-navigation fallback also means that if App.tsx callback wiring is ever lost (e.g. a stray refactor strips the prop), the feature still works — it just isn't architecturally clean.

What still belongs to the epic

With this merged and verified on TestFlight, all 27 child issues of Epic #1446 are complete and the epic can be closed.

Replaces `promotePeekToAmicusThread` (which called `navigation.getParent()`
from the service layer) with a navigation-agnostic `promotePeekToThread`
that returns a discriminated `PromotePeekResult` describing what should
happen next. The actual `navigate(...)` call moves to App.tsx via
`navigationRef`, so the navigation tree shape lives in one place.
This is the re-land of PR #1500 (reverted in #1503 due to the iOS 26
TurboModule crash). The underlying crash mechanism is fixed by
`app/patches/react-native+0.81.5.patch` already in master, so the
specific (12) symptom cannot reoccur.
Architectural improvements vs. the original PR:
- Service has zero React Navigation imports
- Tests don't mock useNavigation/getParent
- Restructuring navigators touches one call site (App.tsx), not service code
- Subsumes the architectural concerns from #1562 / #1563 (closed)
Files:
- NEW app/src/services/amicus/promotePeekToThread.ts
- NEW app/src/services/amicus/__tests__/promotePeekToThread.test.ts
- MOD app/src/services/amicus/studyLaunch.ts (removed deprecated function)
- MOD app/src/services/amicus/__tests__/studyLaunch.test.ts (drop the two cases now in the new file)
- MOD app/src/services/amicus/index.ts (export rename)
- MOD app/src/components/amicus/AmicusFab.tsx (accept onContinueInTab prop, internal nav as defensive fallback)
- MOD app/App.tsx (provide onContinueInTab via navigationRef)
Verification:
- tsc --noEmit: clean
- jest src/services/amicus: 9/9 pass on the changed surface
- eslint: 0 errors on changed files
@github-actions

github-actionsBot commented Apr 27, 2026

Copy link
Copy Markdown

Test Results

✅ All tests passed

PassedFailedTotal
Tests✅ 3962❌ 03962
Suites✅ 529❌ 0529

Coverage

StatementsBranchesFunctionsLines

⏱️ Duration: 94.4s

CI's `expo-doctor — dep version check` step started failing on this
branch (and would fail on master too) after expo-doctor's bundled
'expected versions' map advanced to a newer SDK 54 patch level.
Nine deps were one patch behind:
expo-asset: ~12.0.12 → ~12.0.13
expo-auth-session: ~7.0.10 → ~7.0.11
expo-crypto: ~15.0.8 → ~15.0.9
expo-dev-client: ~6.0.20 → ~6.0.21
expo-file-system: ~19.0.21 → ~19.0.22
expo-notifications: ~0.32.16 → ~0.32.17
expo-screen-orientation: ~9.0.8 → ~9.0.9
expo-updates: ~29.0.16 → ~29.0.17
expo-web-browser: ~15.0.10 → ~15.0.11
`expo` itself was already at ~54.0.34 in package.json; only the
lockfile + transitive deps needed a refresh.
Verification:
- npx expo-doctor@latest: 17/17 checks passed
- tsc --noEmit: clean
- jest src/services/amicus (changed surface): 9/9 pass
- patch-package re-applied react-native+0.81.5.patch cleanly
@CraigBuckmaster
CraigBuckmaster merged commit 51f9e7d into masterApr 27, 2026
6 checks passed
@CraigBuckmaster
CraigBuckmaster deleted the fix/1464-peek-handoff-callback-pattern branch April 27, 2026 20:27
Sign up for freeto 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.

ai-partner: continue-in-tab handoff from peek (re-land with callback pattern)

1 participant

@CraigBuckmaster
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); fix: re-land peek-to-thread handoff with callback pattern (#1464) by CraigBuckmaster · Pull Request #1775 · CraigBuckmaster/ScriptureDeepDive · GitHub
Skip to content

fix: re-land peek-to-thread handoff with callback pattern (#1464) - #1775

Merged
CraigBuckmaster merged 2 commits into
masterfrom
fix/1464-peek-handoff-callback-pattern
Apr 27, 2026
Merged

fix: re-land peek-to-thread handoff with callback pattern (#1464)#1775
CraigBuckmaster merged 2 commits into
masterfrom
fix/1464-peek-handoff-callback-pattern

Conversation

@CraigBuckmaster

Copy link
Copy Markdown
Owner

Closes#1464.

Re-lands the FAB peek → Amicus tab handoff that was originally shipped in PR #1500 and reverted in #1503 due to the iOS 26 TurboModule crash on TestFlight build (12). The underlying crash mechanism (ObjCTurboModule::performVoidMethodInvocation at RCTTurboModule.mm:441__cxa_rethrow → abort) was independently patched by app/patches/react-native+0.81.5.patch (already on master as part of the iOS 26 workaround triangle), so the specific (12) symptom cannot reoccur with the current build configuration.

What changed vs. the original PR

The original promotePeekToAmicusThread called navigation.getParent().navigate('AmicusTab', ...) from inside the service layer. That coupled a leaf component to the tab name and the navigator tree shape — the exact anti-pattern that motivated #1562 / #1563 (now subsumed into this card).

The re-land introduces a clean callback pattern:

  • Service returns a navigation-agnostic PromotePeekResult discriminated union: { kind: 'existing', threadId, ... } or { kind: 'new', promotedMessages, ... }. Zero React Navigation imports.
  • AmicusFab accepts an optional onContinueInTab(result) prop. When the parent provides it, the FAB delegates navigation entirely. When absent (e.g. a test mounting AmicusFab directly), the FAB falls back to internal navigation — defensive, keeps the component usable in isolation.
  • App.tsx provides the concrete navigation via navigationRef. This is now the single call site that knows about 'AmicusTab', 'Thread', and 'NewThread' screen names. If navigators are restructured, only this file changes.

File summary

FileChange
app/src/services/amicus/promotePeekToThread.tsNEW — navigation-agnostic service
app/src/services/amicus/__tests__/promotePeekToThread.test.tsNEW — 7 cases, no nav mocks
app/src/services/amicus/studyLaunch.tsRemoved deprecated promotePeekToAmicusThread + unused imports
app/src/services/amicus/__tests__/studyLaunch.test.tsDropped the two cases now covered in the new test file
app/src/services/amicus/index.tsExport rename
app/src/components/amicus/AmicusFab.tsxNew onContinueInTab? prop; internal nav as defensive fallback
app/App.tsxProvides handleAmicusContinueInTab via navigationRef

Thread-lookup logic (linked question → linked session → latest chapter thread) is preserved verbatim from the previous implementation. Only the navigation side-effect moves out of the service.

Verification

  • tsc --noEmit: clean (0 errors)
  • jest src/services/amicus/__tests__/promotePeekToThread.test.ts src/services/amicus/__tests__/studyLaunch.test.ts: 9/9 pass
  • Broader sweep jest src/services/amicus src/components/amicus: 235/236 pass; the one failure is a pre-existing flaky afterEach timeout in AmicusPeekSheet.test.tsx that passes cleanly when run in isolation (5/5)
  • eslint: 0 errors on changed files

Rollback plan

If this re-introduces a crash on TestFlight, the rollback is mechanical:

  1. git revert <merge-commit> on master
  2. The reverted state restores promotePeekToAmicusThread in studyLaunch.ts, the AmicusFab original handleContinueInTab, and removes the new files
  3. No data migrations or persisted-state changes are involved — this is a pure refactor of the in-process control flow

The FAB's defensive internal-navigation fallback also means that if App.tsx callback wiring is ever lost (e.g. a stray refactor strips the prop), the feature still works — it just isn't architecturally clean.

What still belongs to the epic

With this merged and verified on TestFlight, all 27 child issues of Epic #1446 are complete and the epic can be closed.

Replaces `promotePeekToAmicusThread` (which called `navigation.getParent()`
from the service layer) with a navigation-agnostic `promotePeekToThread`
that returns a discriminated `PromotePeekResult` describing what should
happen next. The actual `navigate(...)` call moves to App.tsx via
`navigationRef`, so the navigation tree shape lives in one place.
This is the re-land of PR #1500 (reverted in #1503 due to the iOS 26
TurboModule crash). The underlying crash mechanism is fixed by
`app/patches/react-native+0.81.5.patch` already in master, so the
specific (12) symptom cannot reoccur.
Architectural improvements vs. the original PR:
- Service has zero React Navigation imports
- Tests don't mock useNavigation/getParent
- Restructuring navigators touches one call site (App.tsx), not service code
- Subsumes the architectural concerns from #1562 / #1563 (closed)
Files:
- NEW app/src/services/amicus/promotePeekToThread.ts
- NEW app/src/services/amicus/__tests__/promotePeekToThread.test.ts
- MOD app/src/services/amicus/studyLaunch.ts (removed deprecated function)
- MOD app/src/services/amicus/__tests__/studyLaunch.test.ts (drop the two cases now in the new file)
- MOD app/src/services/amicus/index.ts (export rename)
- MOD app/src/components/amicus/AmicusFab.tsx (accept onContinueInTab prop, internal nav as defensive fallback)
- MOD app/App.tsx (provide onContinueInTab via navigationRef)
Verification:
- tsc --noEmit: clean
- jest src/services/amicus: 9/9 pass on the changed surface
- eslint: 0 errors on changed files
@github-actions

github-actionsBot commented Apr 27, 2026

Copy link
Copy Markdown

Test Results

✅ All tests passed

PassedFailedTotal
Tests✅ 3962❌ 03962
Suites✅ 529❌ 0529

Coverage

StatementsBranchesFunctionsLines

⏱️ Duration: 94.4s

CI's `expo-doctor — dep version check` step started failing on this
branch (and would fail on master too) after expo-doctor's bundled
'expected versions' map advanced to a newer SDK 54 patch level.
Nine deps were one patch behind:
expo-asset: ~12.0.12 → ~12.0.13
expo-auth-session: ~7.0.10 → ~7.0.11
expo-crypto: ~15.0.8 → ~15.0.9
expo-dev-client: ~6.0.20 → ~6.0.21
expo-file-system: ~19.0.21 → ~19.0.22
expo-notifications: ~0.32.16 → ~0.32.17
expo-screen-orientation: ~9.0.8 → ~9.0.9
expo-updates: ~29.0.16 → ~29.0.17
expo-web-browser: ~15.0.10 → ~15.0.11
`expo` itself was already at ~54.0.34 in package.json; only the
lockfile + transitive deps needed a refresh.
Verification:
- npx expo-doctor@latest: 17/17 checks passed
- tsc --noEmit: clean
- jest src/services/amicus (changed surface): 9/9 pass
- patch-package re-applied react-native+0.81.5.patch cleanly
@CraigBuckmaster
CraigBuckmaster merged commit 51f9e7d into masterApr 27, 2026
6 checks passed
@CraigBuckmaster
CraigBuckmaster deleted the fix/1464-peek-handoff-callback-pattern branch April 27, 2026 20:27
Sign up for freeto 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.

ai-partner: continue-in-tab handoff from peek (re-land with callback pattern)

1 participant

@CraigBuckmaster