Integrate motivation and overview figure generation#16
Merged
Conversation
|
|
1 similar comment
|
|
There was a problem hiding this comment.
Pull request overview
This PR updates DeepGraph’s figure generation pipeline to route “motivation/overview” diagrams through the PaperBanana postwriting-style external diagram path by default (with opt-out), strengthens the PaperBanana prompt for those figures, and adds regression tests for the new default routing behavior.
Changes:
- Add motivation/overview detection + default PaperBanana routing (with env-var opt-out) and new native symbolic fallback renderers.
- Strengthen PaperBanana wrapper prompting for motivation/overview figures; add a Gemini-native fallback image generation path.
- Add unit tests covering default Banana routing and the native opt-out path.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
agents/paperorchestra/figure_orchestra.py |
Adds motivation/overview detection, default external routing, native symbolic renderers, and postwriting-stage diagram-plan augmentation. |
scripts/paperbanana_wrapper.py |
Adds motivation/overview prompt specialization and Gemini-native image-generation fallback when PaperBanana isn’t installed. |
tests/test_evidence_planner.py |
Adds regression tests for default Banana routing and explicit opt-out to native rendering. |
.gitignore |
Ignores additional local tooling caches (.tools/, .uv-cache/). |
Comments suppressed due to low confidence (2)
agents/paperorchestra/figure_orchestra.py:1062
run_postwriting_api_figure_stage()currently pulls entries intodiagram_plannot only whenplot_type == 'diagram', but also when certain tokens appear (e.g., "method", "problem"). This can sweep in normal plot figures like “Benchmark method comparison” and route them through_run_external_diagram()as if they were diagrams. Recommend restricting this selection toplot_type == 'diagram'(or explicitly normalizing selected rows toplot_type='diagram'only when that’s intended).
diagram_plan = [
row
for row in candidate_plan
if str(row.get("plot_type") or "").lower() == "diagram"
or any(
token in " ".join(str(row.get(k) or "") for k in ("figure_id", "title", "objective")).lower()
for token in ("framework", "overview", "method", "problem", "gating", "architecture")
)
]
agents/paperorchestra/figure_orchestra.py:986
force_bananabypassesallow_external_diagrams(it can invoke_run_external_diagram()even whenallow_external_diagrams=False). This makes the parameter’s behavior non-intuitive for callers and effectively removes the ability to globally disable external rendering for motivation/overview diagrams. Consider either (a) honoringallow_external_diagramsinforce_banana, or (b) renaming/adjusting the API to reflect that motivation/overview diagrams are always eligible for external rendering when enabled.
force_banana = (
_banana_motivation_overview_enabled()
and paperbanana_cmd
and _is_motivation_or_overview_figure(fig)
)
prefer_ai = os.getenv("DEEPGRAPH_PAPERBANANA_PREFER_AI", "").strip().lower() in {"1", "true", "yes"}
if force_banana or (allow_external_diagrams and prefer_ai and paperbanana_cmd):
asset = _run_external_diagram(
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+761
to
+768
| if _is_motivation_or_overview_figure(fig): | ||
| if "motivation" in text: | ||
| _render_symbolic_motivation(fig, state, out_path) | ||
| renderer = "symbolic_motivation" | ||
| else: | ||
| _render_symbolic_overview(fig, state, out_path) | ||
| renderer = "symbolic_overview" | ||
| return _native_asset(fid=fid, fig=fig, out_path=out_path, kind="diagram", renderer=renderer, objective=objective) |
Comment on lines
+298
to
+311
| def _is_motivation_overview_spec(spec: dict[str, Any]) -> bool: | ||
| fig = spec.get("figure") or {} | ||
| text = " ".join( | ||
| str(part or "") | ||
| for part in ( | ||
| fig.get("figure_id"), | ||
| fig.get("title"), | ||
| fig.get("objective"), | ||
| spec.get("state_title"), | ||
| spec.get("problem_statement"), | ||
| ) | ||
| ).lower() | ||
| return any(token in text for token in ("motivation", "overview", "teaser", "problem-method-result", "problem method result")) | ||
|
|
Comment on lines
+79
to
+99
| def test_motivation_overview_diagram_uses_banana_by_default(self): | ||
| outline = { | ||
| "plotting_plan": [ | ||
| { | ||
| "figure_id": "fig_motivation_overview", | ||
| "plot_type": "diagram", | ||
| "title": "Motivation overview", | ||
| "objective": "Motivation and overview for selective reasoning.", | ||
| } | ||
| ] | ||
| } | ||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| manifest = run_figure_orchestra( | ||
| outline=outline, | ||
| state={"title": "Selective reasoning"}, | ||
| iterations=[], | ||
| figures_dir=Path(tmpdir), | ||
| baseline=None, | ||
| metric_name="accuracy", | ||
| paperbanana_cmd="printf x > {output}", | ||
| ) |
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
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
Validation