fix(shader): decode reflected value lanes

[BANANASJIM]

Summary

• centralize ShaderValue lane selection for reflected shader variables
• decode reflected f16/f32/f64 and signed/unsigned 8/16/32/64 value lanes consistently in cbuffer, shader constants, and debug output paths
• keep missing-lane fallbacks type-stable
• add mock/test coverage for numeric and named lane forms
• ignore local Astro generated directories

Scope

Quality follow-up after #265/#266. This does not change VS-IN OBJ export behavior and does not add full IA attribute-table export.

Review

• independent reviewer: no blockers
• Qodo: previous fallback finding resolved; latest review reports Bugs (0)
• CodeRabbit: status check passing
• Greptile: non-actionable trial-ended comments only

Residual non-blocking notes: bool maps through u32v and is labeled as uint in debug formatting; very large u64 values are emitted as JSON numbers, so JavaScript clients may lose precision above 2^53 - 1.

Tests

• pixi run check
• pre-commit pixi run check
• pre-push pixi run e2e
• GitHub CI on fe10bf89: lint/typecheck/audit/AUR/tests/build verify passing; macOS and release jobs skipped by workflow rules

[greptile-apps]

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

[qodo-code-review]

PR Summary by Qodo

Fix cbuffer/debug decode for double shader values (f64v) and centralize lane mapping

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Centralize ShaderValue lane selection for reflected shader variable types.
• Decode double/f64 values via f64v across cbuffer, constants, and debug formatting.
• Add unit tests and mocks covering numeric and named double types.

Diagram

graph TD
  A["buffer handler"] --> B["_shader_value_lane_name"] --> C{{"RenderDoc ShaderValue"}}
  D["debug handler"] --> B
  E["helpers _flatten_shader_var"] --> B
  F(("unit tests")) --> A
  F --> D
  F --> G["mock_renderdoc ShaderValue"] --> C

  subgraph Legend
    direction LR
    _m["Module"] ~~~ _t(("Tests")) ~~~ _e{{"External/SDK"}}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use RenderDoc type metadata/enums directly

• ➕ Avoids heuristic string matching (e.g., 'double', 'uint')
• ➕ More robust across API changes and unusual type names
• ➖ May require deeper coupling to RenderDoc reflection types and version differences
• ➖ Harder to test with lightweight mocks

2. Create a single decode function returning typed Python scalars/lists

• ➕ Eliminates repeated lane extraction logic across handlers
• ➕ Central place to add future int width / half / bool handling
• ➖ Larger refactor; higher risk and more churn than this focused fix
• ➖ May require changing handler output contracts

Recommendation: The chosen approach (central lane mapping via _shader_value_lane_name and reusing it across handlers) is the right scope for a follow-up quality fix: it removes duplication and unblocks double decoding with minimal API impact. Consider migrating from heuristic name matching to RenderDoc-provided type metadata if/when broader type coverage (additional integer widths, half, bool) becomes in-scope.

Files changed (7) +137 / -31

Bug fix (2) +32 / -16

_helpers.pyCentralize ShaderValue lane mapping and use it during variable flattening +24/-7

Centralize ShaderValue lane mapping and use it during variable flattening

• Adds '_shader_value_lane_name()' to map reflected variable types (numeric or named) to the correct ShaderValue lane, including 'f64v' for doubles. Updates '_flatten_shader_var' to use the shared mapping instead of local string-based logic.

src/rdc/handlers/_helpers.py

debug.pyDecode and label double values in debug formatting +8/-9

Decode and label double values in debug formatting

• Uses '_shader_value_lane_name()' to select the correct ShaderValue lane when formatting values. Updates type formatting to return "double" when the selected lane is 'f64v'.

src/rdc/handlers/debug.py

Refactor (1) +2 / -14

buffer.pyReuse shared lane mapping for buffer/shader variable decoding +2/-14

Reuse shared lane mapping for buffer/shader variable decoding

• Imports and uses '_shader_value_lane_name()' to replace the duplicated '_shader_variable_value_kind' mapping logic. This enables consistent handling of doubles and keeps type-to-lane selection uniform across handlers.

src/rdc/handlers/buffer.py

Tests (4) +103 / -1

mock_renderdoc.pyExtend ShaderValue mock with f64v lane +1/-0

Extend ShaderValue mock with f64v lane

• Adds an 'f64v' field to the ShaderValue mock to mirror RenderDoc’s union lanes. This enables unit tests to construct and validate double values end-to-end.

tests/mocks/mock_renderdoc.py

test_buffer_decode.pyAdd cbuffer decode tests for double types (numeric and named) +44/-0

Add cbuffer decode tests for double types (numeric and named)

• Introduces tests verifying that RenderDoc numeric type '1' and named type "double" extract values from 'f64v'. Confirms both list (vector) and scalar decoding behavior in the cbuffer decode handler.

tests/unit/test_buffer_decode.py

test_daemon_shader_api_fix.pyValidate shader_constants flattens double variables from f64v +37/-0

Validate shader_constants flattens double variables from f64v

• Adds a unit test that wires a mock pipeline/reflection and ensures shader constants output flattens a double variable using 'f64v'. Confirms the returned type and value match expected double decoding.

tests/unit/test_daemon_shader_api_fix.py

test_debug_handlers.pyAdd debug formatting tests for double values (named and numeric) +21/-1

Add debug formatting tests for double values (named and numeric)

• Extends the test helper to populate 'f64v' and allows non-string var_type inputs. Adds tests asserting '_format_var_value' and '_format_var_type' correctly treat both "double" and numeric type '1' as doubles.

tests/unit/test_debug_handlers.py

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds shared lane-name selection for reflected shader values, applies it in buffer and debug handlers, extends mocks and tests for f64v, and updates docs-astro ignore rules.

Changes

Double-precision shader lane support

Layer / File(s)	Summary
Shared lane-name helper and flatten logic src/rdc/handlers/_helpers.py	Adds _shader_value_lane_name(var_type) and _shader_value_lane_fallback(lane_name), then refactors _flatten_shader_var to use them for reflected value extraction.
Buffer decoding uses shared lane helper src/rdc/handlers/buffer.py, tests/unit/test_buffer_decode.py	buffer.py delegates lane selection to the shared helper, and tests cover double cbuffer decoding for numeric and named types.
Debug formatter uses shared lane helper src/rdc/handlers/debug.py, tests/mocks/mock_renderdoc.py, tests/unit/test_daemon_shader_api_fix.py, tests/unit/test_debug_handlers.py	debug.py uses the shared helper for value extraction and type mapping; the mock ShaderValue adds f64v, and tests cover double formatting plus missing-lane fallback behavior.
docs-astro ignore rules .gitignore	Ignores docs-astro/.astro/ and docs-astro/node_modules/ in addition to docs-astro/dist/.

Estimated code review effort: 2 (Simple) | ~15 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant BufferOrDebugHandler
  participant SharedHelper
  Caller->>BufferOrDebugHandler: decode/format ShaderVariable
  BufferOrDebugHandler->>SharedHelper: _shader_value_lane_name(var_type)
  SharedHelper-->>BufferOrDebugHandler: lane name
  BufferOrDebugHandler-->>Caller: extracted value / formatted type

Loading

Possibly related PRs

• BANANASJIM/rdc-cli#265: Both PRs update shader reflected-value lane selection in src/rdc/handlers/buffer.py around the same RenderDoc value extraction path.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: refactoring shader value lane decoding for reflected values.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/cbuffer-double-and-ignore-docs-artifacts

---

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.}

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Integer fallback emits floats ✓ Resolved 🐞 Bug ≡ Correctness

Description

In _flatten_shader_var, missing u32v/s32v lanes fall back to [0.0]*16, so integer shader variables
can be emitted as floats when the expected lane is absent. This makes the JSON value type unstable
for integer-typed variables and can break downstream consumers that expect ints.

Code

src/rdc/handlers/_helpers.py[R554-555]

+        lane_name = _shader_value_lane_name(getattr(var, "type", ""))
+        values = list(getattr(val, lane_name, [0.0] * 16)[:count])

Evidence

The lane selector can return integer lanes (u32v/s32v), but the flattening code always falls back to
float zeros when the selected attribute is missing. The mocked ShaderValue definition shows
u32v/s32v are integer lists, so float fallbacks are type-incorrect for those lanes.

src/rdc/handlers/_helpers.py[511-530]
src/rdc/handlers/_helpers.py[550-556]
tests/mocks/mock_renderdoc.py[1001-1009]

Agent prompt

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

### Issue description
`_flatten_shader_var()` uses a float fallback (`[0.0] * 16`) for *all* ShaderValue lanes. When `lane_name` resolves to an integer lane (`u32v`/`s32v`) and that attribute is missing on the value object, the fallback produces floats instead of ints, making the JSON schema inconsistent.

### Issue Context
`_shader_value_lane_name()` may return `u32v` or `s32v` for integer types, and the mock RenderDoc API models those lanes as `list[int]`.

### Fix Focus Areas
- src/rdc/handlers/_helpers.py[511-556]

### Suggested change
Select the fallback based on lane:
- if `lane_name in ("u32v", "s32v")`: use `[0] * 16`
- else: use `[0.0] * 16`

Then keep slicing `[:count]` as before.

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

---

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/rdc/handlers/_helpers.py`:
- Around line 511-532: Update _shader_value_lane_name so it explicitly maps
64-bit VarType integer codes 7 and 8 to the correct ShaderValue lanes s64v and
u64v instead of falling back to f32v. Also extend the string-name matching in
the same function to recognize sbyte and ubyte alongside the existing
integer/uint checks, while preserving the current handling for the other numeric
types.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7bc89b40-4098-43ca-b43a-f87d9119fd30

📥 Commits

Reviewing files that changed from the base of the PR and between 73d1c65 and 490f375.

📒 Files selected for processing (8)

• .gitignore
• src/rdc/handlers/_helpers.py
• src/rdc/handlers/buffer.py
• src/rdc/handlers/debug.py
• tests/mocks/mock_renderdoc.py
• tests/unit/test_buffer_decode.py
• tests/unit/test_daemon_shader_api_fix.py
• tests/unit/test_debug_handlers.py

[greptile-apps]

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

[greptile-apps]

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

[greptile-apps]

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