Skip to content

Conversation

@tzanko-matev
Copy link
Contributor

This is the last PR involved with module-name resolution. We end-up relying on __name__ to compute the module name.

- canonicalise sys.path roots and normalise empty entries to the cwd so path-based lookups are stable
- add project-marker heuristics to derive dotted names when sys.modules misses a match and extend coverage to enforce behaviour
- prefer sys.modules names over filesystem-shaped fallbacks and let trace filters choose the most specific project root
design-docs/adr/0016-module-name-resolution-via-globals-name.md: 
design-docs/module-name-resolution-deep-review.md: 
design-docs/module-name-resolution-globals-implementation-plan.md: 

Signed-off-by: Tzanko Matev <tsanko@metacraft-labs.com>
@tzanko-matev
Copy link
Contributor Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

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

Comment on lines +189 to +193
}
Err(_) => None,
};
self.filter.set_module_name_hint(code.id(), globals_name);

Choose a reason for hiding this comment

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

P1 Badge Honour module-name-from-globals policy when caching hints

The new module_name_from_globals switch is meant to let users fall back to the filesystem based resolver, but the tracer always captures the frame’s __name__ and stores it as a hint (set_module_name_hint). FilterCoordinator::resolve then passes that hint to TraceFilterEngine unconditionally, so filtering decisions and cached module names still rely on __name__ even when the flag is disabled, and the costly frame capture happens regardless. As a result, --no-module-name-from-globals has no effect beyond skipping the early return in derive_module_name, leaving users without a way to disable the new behaviour. Consider gating both the capture and the hint propagation on the policy flag.

Useful? React with 👍 / 👎.

@github-actions
Copy link

Coverage Summary

Rust (lines)
82.0% covered (8,237 / 10,045 | 1,808 missed)

File Lines Miss Cover
codetracer-python-recorder/src/logging/logger.rs 273 64 76.6%
codetracer-python-recorder/src/module_identity.rs 181 28 84.5%
codetracer-python-recorder/src/monitoring/api.rs 165 152 7.9%
codetracer-python-recorder/src/monitoring/callbacks.rs 589 68 88.5%
codetracer-python-recorder/src/runtime/frame_inspector.rs 148 63 57.4%
codetracer-python-recorder/src/runtime/io_capture/fd_mirror/unix.rs 376 79 79.0%
codetracer-python-recorder/src/runtime/io_capture/proxies/common.rs 159 62 61.0%
codetracer-python-recorder/src/runtime/io_capture/proxies/input.rs 128 91 28.9%
codetracer-python-recorder/src/runtime/io_capture/proxies/output.rs 41 27 34.1%
codetracer-python-recorder/src/runtime/tracer/events.rs 422 191 54.7%
codetracer-python-recorder/src/runtime/tracer/filtering.rs 144 38 73.6%
codetracer-python-recorder/src/runtime/tracer/io.rs 158 31 80.4%
codetracer-python-recorder/src/runtime/tracer/lifecycle.rs 279 72 74.2%
codetracer-python-recorder/src/runtime/tracer/runtime_tracer.rs 2,082 82 96.1%
codetracer-python-recorder/src/runtime/value_capture.rs 267 85 68.2%
codetracer-python-recorder/src/session.rs 85 85 0.0%
codetracer-python-recorder/src/session/bootstrap/filesystem.rs 90 32 64.4%
codetracer-python-recorder/src/trace_filter/engine.rs 585 52 91.1%
codetracer-python-recorder/src/trace_filter/loader.rs 445 202 54.6%
codetracer-python-recorder/src/trace_filter/selector.rs 258 66 74.4%
Showing top 20 entries by missed lines (of 53 total).

Python (statements)
78.3% covered (281 / 359 | 78 missed)

File Stmts Miss Cover
codetracer-python-recorder/codetracer_python_recorder/__init__.py 7 0 100.0%
codetracer-python-recorder/codetracer_python_recorder/__main__.py 2 2 0.0%
codetracer-python-recorder/codetracer_python_recorder/api.py 5 0 100.0%
codetracer-python-recorder/codetracer_python_recorder/auto_start.py 24 2 91.7%
codetracer-python-recorder/codetracer_python_recorder/cli.py 145 62 57.2%
codetracer-python-recorder/codetracer_python_recorder/formats.py 13 1 92.3%
codetracer-python-recorder/codetracer_python_recorder/session.py 111 5 95.5%
codetracer-python-recorder/codetracer_python_recorder/trace_balance.py 52 6 88.5%

Generated automatically via generate_coverage_comment.py.

Base automatically changed from toplevel-return-event to main October 28, 2025 09:48
@tzanko-matev tzanko-matev merged commit 8400d79 into main Oct 28, 2025
3 checks passed
@tzanko-matev tzanko-matev deleted the refactor-module-name-resolution branch October 28, 2025 09:48
Sign up for free to 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.

2 participants