Add function tracing to SnapshotTracer by rachelzUT · Pull Request #1366 · pyta-uoft/pyta

rachelzUT · 2026-07-03T21:54:34Z

Proposed Changes

Modified SnapshotTracer to trace into calls of functions defined in the same module that SnapshotTracer is called, as SnapshotTracer previously only traced lines of code within the context block and did not trace into any function calls within the block. Note that built-in functions and functions from external libraries/modules are not traced into.

...

Screenshots of your changes (if applicable)

Type of Change

Type	Applies?
🚨 Breaking change (fix or feature that would cause existing functionality to change)
✨ New feature (non-breaking change that adds functionality)	X
🐛 Bug fix (non-breaking change that fixes an issue)
♻️ Refactoring (internal change to codebase, without changing functionality)
🚦 Test update (change that only adds or modifies tests)
📚 Documentation update (change that only updates documentation)
📦 Dependency update (change that updates a dependency)
🔧 Internal (change that only affects developers or continuous integration)

Checklist

Before opening your pull request:

I have performed a self-review of my changes.
- Check that all changed files included in this pull request are intentional changes.
- Check that all changes are relevant to the purpose of this pull request, as described above.
I have added tests for my changes, if applicable.
- This is required for all bug fixes and new features.
I have updated the project documentation, if applicable.
- This is required for new features.
I have updated the project Changelog (this is required for all changes).
If this is my first contribution, I have added myself to the list of contributors.

After opening your pull request:

I have verified that the CI tests have passed.
I have reviewed the test coverage changes reported by Coveralls.
I have requested a review from a project maintainer.

Questions and Comments

I modified the webstepper documentation packages/python-ta/docs/debug/index.md with the new feature. Before it was listed under "Current Limitations" which I just edited to reflect the change, but if it's no longer considered a 'limitation' since this fixes that limitation, I can move the description elsewhere.
The same change to the MemoryViz Webstepper code (handled by the MemoryViz team) is required for this PR as well as for Add webstepper_options attribute and line_context option to SnapshotTracer #1360, specifically implementing a startLineNumber which will ensure the correct line numbers and highlighting are shown in the code section of the Webstepper

coveralls · 2026-07-06T22:30:23Z

Coverage Report for CI Build 29204754231

Coverage remained the same at 90.931%

Details

Coverage remained the same as the base build.
Patch coverage: No coverable lines changed in this PR.
No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

Coverage Stats


Relevant Lines:	4058
Covered Lines:	3690
Line Coverage:	90.93%
Coverage Strength:	17.67 hits per line

💛 - Coveralls

david-yz-liu · 2026-07-06T23:03:07Z

+        self._origin_file = os.path.normcase(
+            os.path.abspath(func_frame.f_globals.get("__file__", ""))
+        )
+        sys.settrace(self._trace_func)


I'm not sure why this is necessary; is it not sufficient to have _trace_func return itself when a new function is called?

We still need to set self._trace_func as the argument to register it as the global trace function so that on any “call” events in the future (when a function is called inside the with block), Python uses the return value of self._trace_func as the local trace for the called function. If we only have _trace_func return itself, tracing wouldn't actually be activated for the called functions.

Okay yes, I recall you also explained this in your presentation last week.

In this case it seems to me like we should leave _trace_func unchanged and instead define a separate global tracing function that only handles function calls. This will help keep the code more modular and make clear the two separate purposes.

david-yz-liu · 2026-07-06T23:10:07Z

        func_frame.f_trace = self._trace_func
-        sys.settrace(lambda *_args: None)
+        self._origin_file = os.path.normcase(
+            os.path.abspath(func_frame.f_globals.get("__file__", ""))


"" is not a good default. If the file cannot be found, let's be conservative and do not trace into any function calls.

david-yz-liu · 2026-07-06T23:21:39Z

+                self._traced_frames.append(
+                    {
+                        "co_firstlineno": frame.f_code.co_firstlineno,
+                        "source_lines": inspect.getsource(frame.f_code).splitlines(),


Overall this approach is storing more than it needs.

The existing _first_line attribute can be reused but renamed to _start_lineno; its role should be changed to store the minimum line number that should be displayed. Similarly, you can add an _end_lineno attribute to store the maximum line number that should be displayed. Both can be updated directly here, instead of introducing a new self._trace_frames attribute.

In _get_code below, right now _first_line is being used to index into just the source lines of the original function frame. We can greatly simplify by (1) getting the full lines of the module rather than just the single function, and then (2) directly indexing into that list using _start_lineno and _end_lineno, without doing any offsetting.

david-yz-liu · 2026-07-06T23:23:23Z

    <script>
      window.codeText = `{{ code_text }}`;
      window.memoryVizData = {{ memory_viz_data | tojson }};
+      window.startLineNumber = {{ start_line_number }};


Do not make changes to the webstepper in this PR. This PR should deal entirely with the logical changes necessary to do the tracing into function calls. This is why I said:

To avoid the issue you ran into in the earlier webstepper PR, you could try testing this with only functions that are defined below the SnapshotTracer context block.

david-yz-liu · 2026-07-06T23:24:01Z

+        """Take a snapshot of the variables in the functions specified in `self.include`, tracing into same-module function calls."""
+        if event == "call":
+            # Only trace functions in the same module as the calling function.
+            called_file = os.path.normcase(os.path.abspath(frame.f_globals.get("__file__", "")))


Similar to my comment below, if no __file__ attribute is found, do not trace into the call.

…ineno

david-yz-liu · 2026-07-11T16:03:49Z


 1. Due to differences in Python interpreters, this context manager only works with Python versions >= 3.10.
-2. The context manager does not step into any function calls. Calling functions within the traced function may lead to undefined behavior.
+2. The context manager only steps into Python functions defined in the same module as the traced context. Calls into external modules and builtins are not traced.


This wording change is good, but I think we should move this out of the "limitations" section and instead have this be a general description of the function tracing behaviour for SnapshotTracer.

david-yz-liu · 2026-07-11T16:13:30Z

+        self._origin_file = os.path.normcase(
+            os.path.abspath(func_frame.f_globals.get("__file__", ""))
+        )
+        sys.settrace(self._trace_func)


Okay yes, I recall you also explained this in your presentation last week.

In this case it seems to me like we should leave _trace_func unchanged and instead define a separate global tracing function that only handles function calls. This will help keep the code more modular and make clear the two separate purposes.

david-yz-liu · 2026-07-11T19:11:28Z

+        self._origin_file = None
+        self._module_source_lines = None
+
+    def _global_trace_func(self, frame: types.FrameType, event: str, _arg: Any) -> Any:


Oh I think I missed this last time, but the return type here can be simplified (it's either a Callable or None)

david-yz-liu · 2026-07-11T19:14:48Z

+
+    def _global_trace_func(self, frame: types.FrameType, event: str, _arg: Any) -> Any:
+        """Global trace function that handles 'call' events to determine which functions to trace into."""
+        if event == "call":


By splitting this off into a separate helper you can simplify the logic a bit. I would have one guard at the very start of the function for an early return (if event != "call" or self._origin_file is None).

david-yz-liu · 2026-07-11T19:16:57Z

+            # Only trace functions in the same module as the calling function.
+            called_file = os.path.normcase(os.path.abspath(frame.f_code.co_filename))
+            if called_file == self._origin_file and frame.f_code.co_name not in (
+                "_trace_func",


Oh I missed this too, but please use self._trace_func.__name__, and similarly for the other one. This is better than string constants, as it'll be detected as a usage of the methods, which is good if we ever rename them.

david-yz-liu

Nice work, @rachelzUT!

rachelzUT added 3 commits July 3, 2026 17:40

added function tracing logic

f6ba8fc

updated webstepper code sections to show traced functions

1d7f44e

merge upstream master

bd915aa

rachelzUT marked this pull request as ready for review July 6, 2026 22:21

rachelzUT requested a review from david-yz-liu July 6, 2026 22:30

david-yz-liu reviewed Jul 6, 2026

View reviewed changes

replaced _traced_frames and _first_line with _start_lineno and _end_l…

10e30f0

…ineno

rachelzUT requested a review from david-yz-liu July 10, 2026 17:30

david-yz-liu reviewed Jul 11, 2026

View reviewed changes

added _global_trace_func

67bf8bf

rachelzUT requested a review from david-yz-liu July 11, 2026 17:15

david-yz-liu reviewed Jul 11, 2026

View reviewed changes

changed return types and simplified logic of _global_trace_func

aadae64

rachelzUT requested a review from david-yz-liu July 12, 2026 19:06

david-yz-liu approved these changes Jul 13, 2026

View reviewed changes

david-yz-liu merged commit 728645f into pyta-uoft:master Jul 13, 2026
30 checks passed

Uh oh!

Conversation

rachelzUT commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Type of Change

Checklist

Questions and Comments

Uh oh!

coveralls commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 29204754231

Coverage remained the same at 90.931%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-yz-liu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rachelzUT commented Jul 3, 2026 •

edited

Loading

coveralls commented Jul 6, 2026 •

edited

Loading