Skip to content

Make docs builds fail on notebook execution errors and fix all doc issues #369

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 17 commits into from
Nov 3, 2025
Merged

Make docs builds fail on notebook execution errors and fix all doc issues #369

merged 17 commits into from
Nov 3, 2025

Conversation

Copy link
Contributor

Copilot AI commented Nov 3, 2025 •
edited
Loading

Documentation tutorials used deprecated @egraph.class_ decorator causing AttributeError, but docs builds were passing because nb_execution_raise_on_error = False silently ignored execution failures.

Changes

  • docs/conf.py: Set nb_execution_raise_on_error = True to fail builds on notebook errors
  • docs/conf.py: Exclude conf.py from being processed as a notebook (it's a Sphinx config file)
  • docs/.gitignore: Added docs/sg_execution_times.rst (auto-generated file)
  • docs/tutorials/getting-started.ipynb:
    • Remove @egraph.class_ decorator (classes now inherit directly from Expr)
    • Replace @egraph.function → @function
    • Replace egraph.simplify(expr, n) → egraph.run(n) + egraph.extract(expr)
    • Fix duplicate egraph.run(10) call
  • docs/explanation/*.ipynb (6 files): Fixed all @egraph.class_ and @egraph.function usages
  • docs/reference/high-level.md: Removed @egraph.class_ decorator
  • docs/tutorials/tut_2_datalog.py: Fixed check() to use eq().to() instead of ==
  • docs/explanation/2023_12_02_congruence_closure.md: Removed unsupported @function(default=) syntax

Temporary exclusions

Only 2 notebooks excluded due to unrelated errors (not @egraph.class_ issues):

  • explanation/2024_03_17_community_talk.ipynb - sklearn configuration error
  • explanation/indexing_pushdown.ipynb - array_api_module NameError

Before:

@egraph.class_
class Dim(Expr):
    def __init__(self, value: i64Like) -> None: ...
    
egraph.simplify(res, 10)

After:

class Dim(Expr):
    def __init__(self, value: i64Like) -> None: ...

egraph.run(10)
egraph.extract(res)
Original prompt

This section details on the original issue you should resolve

<issue_title>Broken tutorial in docs</issue_title>
<issue_description>I noticed that the tutorial "Getting Started - Matrix Multiplication" (https://egglog-python.readthedocs.io/latest/tutorials/getting-started.html) uses a decorator @egraph.class_ that seems to no longer exist, causing an attribute error. I think it should be removed, and the syntax for creating expressions and registering rewrite rules should be adjusted.</issue_description>

<agent_instructions> Please fix the builds in docs, but first make sure that failing docs builds will fail in CI. They run make docs so first make sure that make docs fails currently on the broken file, commit that and push it to a branch, open a PR, then commit the fixes to resolve the doc issues so that make docs succeeds again.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@saulshanabrook Thank you! Yeah we should also check why the docs builds are passing even with this error and fix that so these are highlighted more.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

saulshanabrook
saulshanabrook reviewed Nov 3, 2025
View reviewed changes
@codspeed-hq
Copy link

codspeed-hq bot commented Nov 3, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #369 will degrade performances by 11.3%

Comparing copilot/fix-71104912-561003658-9ac438cb-9ef7-4c89-a867-98c9c8bbfad8 (9168d4b) with main (adf9ea3)1

Summary

❌ 1 regression
✅ 13 untouched

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Mode Benchmark BASE HEAD Change
❌ WallTime test_jit[lda] 15 s 16.9 s -11.3%

Footnotes

  1. No successful run was found on main (9168d4b) during the generation of this report, so adf9ea3 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Copilot AI changed the title [WIP] Broken tutorial in docs Make docs builds fail on notebook execution errors and fix getting-started tutorial Nov 3, 2025
Copilot AI requested a review from saulshanabrook November 3, 2025 04:43
Copilot finished work on behalf of saulshanabrook November 3, 2025 04:43
saulshanabrook
saulshanabrook requested changes Nov 3, 2025
View reviewed changes
Copy link
Member

@saulshanabrook saulshanabrook left a comment

Choose a reason for hiding this comment

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

@copilot Can you respond adjust based on this review?

Also, can you change:

egraph.run(10)
print(egraph.extract(x_mult_y.ncols()))
egraph.run(10)
print(egraph.extract(x_mult_y.nrows()))

To just run once, like:

egraph.run(10)
print(egraph.extract(x_mult_y.ncols()))
print(egraph.extract(x_mult_y.nrows()))

Copilot AI and others added 2 commits November 3, 2025 04:58
Copy link
Contributor Author

Copilot AI commented Nov 3, 2025

@copilot Can you respond adjust based on this review?

Also, can you change:...

Done in commit 1d39e0f. Removed the duplicate egraph.run(10) - now it only runs once before extracting both values.

Copilot AI changed the title Make docs builds fail on notebook execution errors and fix getting-started tutorial Make docs builds fail on notebook execution errors and fix all @egraph.class_ issues Nov 3, 2025
Copilot AI requested a review from saulshanabrook November 3, 2025 05:00
Copilot finished work on behalf of saulshanabrook November 3, 2025 05:00
@saulshanabrook
Copy link
Member

saulshanabrook commented Nov 3, 2025

@codex can you fix the remaining two docs here so they work? Can you also run the required formatting so it passes in CI?

@chatgpt-codex-connector
Copy link

chatgpt-codex-connector bot commented Nov 3, 2025

Summary

Testing

  • ✅ make docs

View task →

@saulshanabrook saulshanabrook changed the title Make docs builds fail on notebook execution errors and fix all @egraph.class_ issues Make docs builds fail on notebook execution errors and fix all doc issues Nov 3, 2025
@saulshanabrook
Copy link
Member

saulshanabrook commented Nov 3, 2025

@actions-user changelog

actions-user and others added 2 commits November 3, 2025 21:18
@actions-user
Add changelog entry for PR #369
4c1344d
@saulshanabrook
Merge pull request #372 from egraphs-good/codex/github-mention-make-d…
b33d55e 
…ocs-builds-fail-on-notebook-execution

Fix docs execution for community talk and indexing notebooks
@saulshanabrook saulshanabrook marked this pull request as ready for review November 3, 2025 21:24
@saulshanabrook saulshanabrook force-pushed the copilot/fix-71104912-561003658-9ac438cb-9ef7-4c89-a867-98c9c8bbfad8 branch from 6dd4810 to 4bcad9a Compare November 3, 2025 21:27
@chatgpt-codex-connector
Copy link

chatgpt-codex-connector bot commented Nov 3, 2025

💡 Codex Review

egglog-python/src/egraph.rs

Lines 67 to 71 in 6dd4810

match py.detach(|| self.egraph.run_program(commands)) {
Err(e) => Err(WrappedError::Egglog(e)),
Ok(outputs) => {
if let Some(err) = PyErr::take(py) {
return Err(WrappedError::Py(err));

P1 Badge Propagate pending PyErr when command execution fails

The new run_program branch returns WrappedError::Egglog(e) immediately when self.egraph.run_program fails, but it never checks for a pending Python exception with PyErr::take. If a user-defined Python callback invoked during command execution raises (common for cost functions or extraction hooks), the Python traceback is now dropped and replaced by a generic EggSmolError, making the original cause invisible to callers. The prior implementation always consumed PyErr::take(py) even on the error path to surface Python-side failures. Consider checking PyErr::take(py) before returning so that Python exceptions are propagated instead of being masked.

ℹ️ 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".

@saulshanabrook saulshanabrook merged commit 9ccb5b4 into main Nov 3, 2025
20 checks passed
@saulshanabrook saulshanabrook deleted the copilot/fix-71104912-561003658-9ac438cb-9ef7-4c89-a867-98c9c8bbfad8 branch November 3, 2025 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@saulshanabrook saulshanabrook Awaiting requested review from saulshanabrook

Assignees

@saulshanabrook saulshanabrook

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Broken tutorial in docs

3 participants

@saulshanabrook @actions-user