source-academy / py-slang / 30791743260

86%

push

github

Add regression test for CodeRabbit's ModuleFunction/Opaque substitution concern (#387)

CodeRabbit flagged that substitute()'s generic mapChildren/clone() walk
might corrupt ModuleFunction.closure / Opaque.handle during AST
substitution, since neither field is special-cased as an opaque leaf.

Verified this is a false positive: mapValue only recurses into a field
whose own `.type` is a string (DataType.CLOSURE/OPAQUE are numbers, so
already skipped); clone() does deep-copy the wrapper object, but
ClosureIdentifier/OpaqueIdentifier are `Identifier & {...}` and
conductor's own types define Identifier = number, so the brand is
compile-time only and GenericDataHandler's lookups key off that
primitive number, not object identity — a cloned wrapper still
resolves correctly.

Extended the opaque round-trip test to bind the value to a variable
first (forcing it through substitute's clone(), not just nested
directly as a call argument, which bypasses substitute entirely via
plain object-spread rebuilding) to pin this down as a real regression
guard rather than just an assertion in a review reply.


Claude-Session: https://claude.ai/code/session_017w4FqCep4j8XyFpF4kpVue

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

4657 of 5835 branches covered (79.81%)

Branch coverage included in aggregate %.

10378 of 11638 relevant lines covered (89.17%)

167114.03 hits per line