fix mesh vs-in position selection

[BANANASJIM]

Summary

• select VS-IN position inputs by semantic, user override, or vertex-rate float/vector heuristic
• ignore instance-rate inputs and warn when heuristic selection has multiple plausible candidates
• expose selected position metadata in mesh data/JSON and OBJ comments

Scope

Part of #224. This intentionally does not implement full VS-IN/IA attribute table export; that remains separate from OBJ export.

Tests

• uv run --extra dev pytest tests/unit/test_buffer_decode.py tests/unit/test_mesh_commands.py -q
• uv run --extra dev ruff check src/rdc/handlers/buffer.py src/rdc/commands/mesh.py tests/unit/test_buffer_decode.py tests/unit/test_mesh_commands.py
• uv run --extra dev ruff format --check src/rdc/handlers/buffer.py src/rdc/commands/mesh.py tests/unit/test_buffer_decode.py tests/unit/test_mesh_commands.py
• pixi run check
• pre-commit pixi run check

Review

• independent code review completed with no blocking findings
• final follow-up review completed with no blocking findings

Not run

• pixi run e2e / pre-push: local environment is missing .local/renderdoc/renderdoc.so; the hook documents using --no-verify when the RenderDoc module is absent

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.

[qodo-code-review]

PR Summary by Qodo

Fix VS-IN mesh position selection with overrides, heuristics, and metadata export

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Add explicit CLI selectors for VS-IN position attribute (by name, index, or slot/offset).
• Improve VS-IN fallback selection to ignore instance-rate inputs and use float/vector heuristics.
• Surface selected position metadata (and ambiguity warnings) in JSON results and OBJ
 header/comments.

Diagram

graph TD
U["User / CLI"] --> C["mesh_cmd (CLI)"] --> R["RPC: mesh_data"] --> H["_handle_mesh_data"] --> S{"Select position input"} --> D["Decode positions"] --> O["Emit JSON / OBJ"]
O --> O2["OBJ header includes position metadata"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Export and use full IA attribute table for selection

• ➕ More deterministic selection; less reliance on heuristics
• ➕ Enables richer debugging/export (e.g., all attributes, not just position)
• ➖ Larger scope (explicitly out-of-scope for this PR)
• ➖ More output/schema surface area to maintain

2. Use shader reflection / pipeline binding metadata to map position input

• ➕ Potentially more accurate across APIs where semantics/names differ
• ➕ Could disambiguate multiple float/vector candidates without warnings
• ➖ May be unavailable/incomplete in captures and varies by API/backend
• ➖ Higher implementation complexity than current pragmatic heuristic

Recommendation: The PR’s approach is appropriate for the scoped fix: prefer explicit user overrides, fall back to semantic matching, and only then use a constrained heuristic that ignores instance-rate inputs and warns on ambiguity. The main alternative—exporting the full IA attribute table—would be more robust but is a larger, separately-scoped feature.

Files changed (5) +498 / -17

Enhancement (1) +27 / -1

mesh.pyAdd position selector options, warning output, and OBJ header metadata +27/-1

Add position selector options, warning output, and OBJ header metadata

• Introduces new click options to forward position selection parameters to mesh_data. Emits a CLI warning when the backend reports ambiguous heuristic selection, and includes selected position metadata in the OBJ header comment.

src/rdc/commands/mesh.py

Bug fix (1) +152 / -11

buffer.pyImplement robust VS-IN position input selection with overrides and heuristics +152/-11

Implement robust VS-IN position input selection with overrides and heuristics

• Replaces simplistic POSITION-semantic selection with a selector that supports user overrides (by name, index, or slot/offset), semantic matching, and a float/vector heuristic that ignores instance-rate inputs. Returns position metadata (attribute name, source, index, slot/offset, format) and an optional ambiguity warning in the mesh_data result.

src/rdc/handlers/buffer.py

Tests (2) +315 / -5

test_buffer_decode.pyAdd unit coverage for heuristic selection, override validation, and metadata fields +256/-5

Add unit coverage for heuristic selection, override validation, and metadata fields

• Extends VS-IN/IA fallback tests to assert returned position metadata and updated failure messages. Adds new tests for heuristic behavior (ignoring instanced inputs, warning on ambiguity) and for override selectors and validation errors.

tests/unit/test_buffer_decode.py

test_mesh_commands.pyTest CLI forwarding of selectors and printing of position warnings +59/-0

Test CLI forwarding of selectors and printing of position warnings

• Adds tests ensuring mesh_cmd forwards new selector flags to mesh_data, includes them in help output, and prints backend-provided position warnings while still emitting OBJ content.

tests/unit/test_mesh_commands.py

Documentation (1) +4 / -0

commands-quick-ref.mdDocument new VS-IN position selection flags for mesh export +4/-0

Document new VS-IN position selection flags for mesh export

• Adds CLI reference entries for --position-attribute/--position-index/--position-slot/--position-offset to the mesh command quick reference.

src/rdc/_skills/references/commands-quick-ref.md

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 Add export support for shader constant buffers and VS input meshes

1. Instance semantic can be chosen ✓ Resolved 🐞 Bug ≡ Correctness

Description

_select_position_input() returns the first POSITION-like semantic without excluding per-instance
inputs, so an instanced POSITION attribute can be selected and decoded even though instance inputs
are otherwise ignored in the heuristic fallback. This can export incorrect vertex positions when a
pipeline exposes an instanced POSITION-like input alongside vertex-rate attributes.

Code

src/rdc/handlers/buffer.py[R585-594]

+    for index, vi in enumerate(inputs):
+        if _is_position_semantic(vi):
+            _validate_position_input(vi)
+            return vi, "semantic", index, None
+
+    candidates = [
+        (index, vi)
+        for index, vi in enumerate(inputs)
+        if not _is_instance_input(vi) and _is_float_vector_input(vi)
+    ]

Evidence

The semantic selection loop returns on the first POSITION-like semantic without calling
_is_instance_input, while the heuristic fallback explicitly filters out instance inputs using
_is_instance_input—so the behavior is inconsistent and can select instanced semantics.

src/rdc/handlers/buffer.py[493-507]
src/rdc/handlers/buffer.py[585-596]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_select_position_input()` performs a semantic-based fast path selection (`POSITION`/`POS*`) before applying any instance-rate filtering. As a result, instance-rate POSITION-like inputs can be selected even though instance inputs are explicitly excluded in the heuristic candidate list.

## Issue Context
This selection happens in the VS-IN IA fallback path. The code already has `_is_instance_input()` and uses it for heuristic filtering, but not for semantic selection.

## Fix Focus Areas
- src/rdc/handlers/buffer.py[585-614]
- src/rdc/handlers/buffer.py[501-507]

## Suggested fix
Update the semantic selection loop to ignore instance-rate inputs (e.g., `if _is_position_semantic(vi) and not _is_instance_input(vi): ...`). If all semantic matches are instance-rate, proceed to heuristic selection (or raise a clearer error), while still allowing explicit user overrides to select instance inputs if desired.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Misleading POSITION bind error ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

_mesh_data_from_ia() raises "POSITION vertex buffer is not bound" even when the selected position
input was user-overridden to a different attribute (e.g., TEXCOORD). This makes debugging override
failures confusing because the error message names the wrong attribute.

Code

↗ src/rdc/handlers/buffer.py

    fmt = getattr(pos_input, "format", None)

Evidence

The code path explicitly supports selecting pos_input via user overrides, but the subsequent
binding check emits a fixed "POSITION" message regardless of what was selected.

src/rdc/handlers/buffer.py[617-635]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The IA decode path now supports selecting a non-POSITION attribute as the position source, but the vertex-buffer binding error is still hard-coded to "POSITION vertex buffer is not bound".

## Issue Context
After `_select_position_input()` chooses `pos_input`, `_mesh_data_from_ia()` checks whether the corresponding vertex buffer is bound, and raises a ValueError with a hard-coded message.

## Fix Focus Areas
- src/rdc/handlers/buffer.py[623-635]

## Suggested fix
Change the error message to reference the selected input (e.g., `_input_display_name(pos_input)` and/or slot index) such as:
`raise ValueError(f"vertex buffer for {_input_display_name(pos_input)!r} (slot {slot}) is not bound")`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[coderabbitai]

Warning

Review limit reached

@BANANASJIM, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 47 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ef5a797b-beec-4ba5-b9c9-7fe937f1cd11

📥 Commits

Reviewing files that changed from the base of the PR and between 01527a8 and d3c52c4.

📒 Files selected for processing (6)

• docs-astro/src/data/commands.json
• src/rdc/_skills/references/commands-quick-ref.md
• src/rdc/commands/mesh.py
• src/rdc/handlers/buffer.py
• tests/unit/test_buffer_decode.py
• tests/unit/test_mesh_commands.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/issue-224-vsin-position-selection

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.