Fix ReshapeReshape initializer name collisions when resolving shared -1 shapes

[Copilot]

ReshapeReshape could rewrite multiple matches that share the same shape initializer ([2, -1]) into distinct concrete shapes, but reuse the same initializer name. This caused initializer map clobbering and left rewritten Reshape inputs pointing to non-registered values after name-fixup.

• Rewrite rule change (_basic_rules.py)

  • Stop reusing the incoming shape value name for synthesized shape initializers.
  • Generate a per-match initializer name from the matched second reshape node context, then create the new initializer with that unique name.
  • This prevents cross-match overwrite in graph.initializers when multiple chains share one original shape input.

• Regression coverage (_basic_rules_test.py)

  • Add a focused test with two Reshape -> Reshape chains sharing shared_shape=[2, -1], where -1 resolves differently per chain.
  • Run the same optimizer sub-pipeline used in the repro and assert rewritten reshape shape-inputs are distinct and registered initializers.

# before: both rewrites reused shape.name (e.g. "shared_shape")
new_shape = op.initializer(ir.Tensor(self._new_shape, name=shape.name))

# after: per-match unique shape initializer name
self._new_shape_name = f"{context.nodes[1].name or shape.name}/shape"
new_shape = op.initializer(ir.Tensor(self._new_shape, name=self._new_shape_name))

[justinchuby]

@copilot was there a way from the rewrite rule class to get a unique name? No worries if there is not. Just wanted to double check in case there is

[Copilot]

@copilot was there a way from the rewrite rule class to get a unique name? No worries if there is not. Just wanted to double check in case there is

I double-checked: not from RewriteRuleClassBase today. In rewrite rules, op is a TapeBuilder, and it doesn’t expose a unique-name API for initializers/values. The unique-name helper exists in other contexts (e.g. converter internals), but not through the rewrite-rule interface, so this rule used the matched node context to keep names distinct.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.61%. Comparing base (f594680) to head (89af432).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2947      +/-   ##
==========================================
+ Coverage   72.59%   72.61%   +0.01%     
==========================================
  Files         263      263              
  Lines       32007    32034      +27     
  Branches     3012     3013       +1     
==========================================
+ Hits        23236    23261      +25     
- Misses       7748     7749       +1     
- Partials     1023     1024       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

[Copilot]

Pull request overview

This PR addresses a correctness issue in the ReshapeReshape rewrite rule where synthesizing new concrete shape initializers could reuse an existing shape initializer name across multiple matches, leading to initializer-map clobbering and invalid value references after subsequent passes.

Changes:

• Update ReshapeReshape to name synthesized shape initializers using a per-match derived name instead of reusing the incoming shape value’s name.
• Add a regression test covering two Reshape -> Reshape chains sharing a single shared_shape=[2, -1] initializer where -1 resolves differently per chain.
• Exercise the optimizer sub-pipeline used in the repro and assert rewritten Reshape shape inputs are distinct and backed by registered initializers.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
onnxscript/rewriter/rules/common/_basic_rules.py	Adjusts synthesized shape initializer naming for ReshapeReshape to avoid collisions across multiple matches.
onnxscript/rewriter/rules/common/_basic_rules_test.py	Adds regression coverage for shared -1 shape initializer rewrites producing distinct concrete shape initializers.

[bas-aarts]

perfect. I just came here today to file this issue. Hope to see this resolved quickly.

[justinchuby]

@copilot address review comments if reasonable

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[Copilot]

@copilot address review comments if reasonable

Done. Changed the initializer name prefix from context.nodes[1].name or shape.name to context.output_values[0].name, which is guaranteed to be unique per match in the graph. Commit: the latest push.