Skip to content

fix: wire video_presentation fan-in as a barrier so generate_slide_scene_codes fires once#1538

Closed
mvanhorn wants to merge 1 commit into
MODSetter:mainfrom
mvanhorn:fix/1531-video-presentation-fanin-barrier
Closed

fix: wire video_presentation fan-in as a barrier so generate_slide_scene_codes fires once#1538
mvanhorn wants to merge 1 commit into
MODSetter:mainfrom
mvanhorn:fix/1531-video-presentation-fanin-barrier

Conversation

@mvanhorn

@mvanhorn mvanhorn commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

The video-presentation agent graph now uses a LangGraph list-based fan-in edge so the slide-generation node runs once after all upstream nodes complete, instead of once per incoming edge.

Description

The presentation node was wired to multiple upstream nodes with separate edges, so LangGraph triggered it once per completed predecessor rather than as a single join, producing duplicate slide-generation passes. This switches those edges to a list-based fan-in edge, which creates a proper barrier so the node runs exactly once.

Motivation and Context

Fixes #1531

Screenshots

N/A — backend agent-graph change with no visual surface.

API Changes

  • This PR includes API changes

Change Type

  • Bug fix
  • New feature
  • Performance improvement
  • Refactoring
  • Documentation
  • Dependency/Build system
  • Breaking change

Testing Performed

  • Tested locally
  • Manual/QA verification

Added unit tests under surfsense_backend/tests/unit/agents/ that drive the graph and assert the fan-in node is triggered exactly once.

Checklist

  • Follows project coding standards and conventions

High-level PR Summary

This PR fixes a bug in the video presentation agent graph where the slide scene generation node was being triggered multiple times instead of once. The fix replaces two separate edges with a single list-based fan-in edge that creates a proper barrier, ensuring generate_slide_scene_codes runs exactly once after both create_slide_audio and assign_slide_themes complete. Unit tests were added to verify the barrier behavior and confirm the node is triggered only once.

⏱️ Estimated Review Time: 15-30 minutes

💡 Review Order Suggestion
Order File Path
1 surfsense_backend/app/agents/video_presentation/graph.py
2 surfsense_backend/tests/unit/agents/test_video_presentation_graph.py

Need help? Join our Discord

Summary by CodeRabbit

  • Bug Fixes

    • Updated the video presentation workflow so scene code generation waits for both required preparation steps to finish before running.
    • Prevented scene code generation from running more than once when branches complete at different times.
  • Tests

    • Added unit coverage for the new workflow behavior, including execution order and single-trigger validation.
    • Added a state-based test to confirm the join behavior remains consistent across staggered completion timing.

@vercel

vercel Bot commented Jun 24, 2026

Copy link
Copy Markdown

@mvanhorn is attempting to deploy a commit to the Rohan Verma's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 205e1cd5-4017-4174-8146-c3ee782206f8

📥 Commits

Reviewing files that changed from the base of the PR and between ee241e0 and fb42356.

📒 Files selected for processing (2)
  • surfsense_backend/app/agents/video_presentation/graph.py
  • surfsense_backend/tests/unit/agents/test_video_presentation_graph.py

📝 Walkthrough

Walkthrough

The PR fixes a latent double-execution bug in the video presentation LangGraph agent: two independent add_edge calls into generate_slide_scene_codes are replaced with a single list-source barrier edge. A new test module is added with three unit tests validating that the node fires exactly once and only after both upstream branches complete.

Changes

Video Presentation Graph Barrier Fix

Layer / File(s) Summary
Barrier edge wiring
surfsense_backend/app/agents/video_presentation/graph.py
Replaces two separate add_edge("create_slide_audio", ...) and add_edge("assign_slide_themes", ...) calls with a single add_edge(["create_slide_audio", "assign_slide_themes"], "generate_slide_scene_codes"), enforcing wait-for-all semantics.
Test module setup and helpers
surfsense_backend/tests/unit/agents/test_video_presentation_graph.py
Introduces module/node constants, pytestmark, _DelayedState typed dict, and _config/_input_state builders used across all tests.
Monkeypatched graph construction
surfsense_backend/tests/unit/agents/test_video_presentation_graph.py
Implements _build_video_graph, which creates a fake nodes module with async stubs, monkeypatches sys.modules, reimports build_graph(), and returns a compiled graph with tracked call order.
Barrier correctness and wiring tests
surfsense_backend/tests/unit/agents/test_video_presentation_graph.py
Three tests assert: scene codes fire exactly once with correct ordering; the compiled graph registers a single join-channel trigger matching the barrier sources; the barrier fires once even when one branch finishes a superstep later.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Two edges once raced to the scene,
firing twice — what a terrible dream!
Now a barrier holds, one list to rule,
both branches must finish — that's the new rule.
No double LLM calls, no doubled render pain,
the rabbit hops once down the graph's clean lane! 🎬

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main fix: converting video_presentation fan-in to a barrier so scene code generation runs once.
Linked Issues check ✅ Passed The graph change and added tests match issue #1531 by enforcing wait-for-all barrier semantics and single execution of generate_slide_scene_codes.
Out of Scope Changes check ✅ Passed The PR stays focused on the graph wiring bug and validation tests, with no evident unrelated code changes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@MODSetter

Copy link
Copy Markdown
Owner

@mvanhorn Can you reopen this on dev branch?

@mvanhorn

Copy link
Copy Markdown
Contributor Author

Reopened against dev as #1563, per your request. Thanks @MODSetter!

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

Labels

None yet

Projects

None yet

2 participants