Skip to content

fix(worker): relabel rewriter tags after cross-backend reshape (#113)#119

Merged
Varashi merged 1 commit into
mainfrom
fix/113-relabel-cross-backend-tags
May 30, 2026
Merged

fix(worker): relabel rewriter tags after cross-backend reshape (#113)#119
Varashi merged 1 commit into
mainfrom
fix/113-relabel-cross-backend-tags

Conversation

@Varashi
Copy link
Copy Markdown
Owner

@Varashi Varashi commented May 29, 2026

Summary

Closes #113. After a cross-backend reshape onto NVENC, the rewriter executes the CUDA branch (`inlineass(CUDA)`, `tonemap_cuda`) but the emitted tags still showed VAAPI-canonical names — so the 320/320 nvenc qa_matrix pass logged `hw-decode:filter:opencl-tonemap->vaapi:inlineass-vaapi` even though the actual filter graph was `tonemap_cuda + inlineass(CUDA)`.

The output was correct; the tag strings misrepresented the executed graph.

What changed

  • New `relabelCrossBackendTags(tags)` in `worker/agent/rewriter_tags.go` — detects `cross-backend:*->X` and rewrites VAAPI-named substrings (`inlineass-vaapi`, `bitmap-inlineass-vaapi`, `opencl-tonemap->vaapi`, `tonemap_opencl-normalized`) to the X target's names. NVENC mapping ships; same-backend / unknown-target are no-ops.
  • Called from `worker/agent/main.go` right before `log.Printf("rewriter applied: ...")` — only the log + matrix consumers see the tags, so a string-level rewrite is safe.
  • Unit tests in `rewriter_tags_relabel_test.go` cover the prod-shape tag list, bitmap+HDR-tonemap prefix tag, SW-tonemap normalized tag, and the three no-op cases (no marker / target=vaapi / unknown target).

Test plan

  • `go build ./...` clean
  • `go test` — new tests + existing `TestRewriterTagInventory` green
  • CodeRabbit pass

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced tag post-processing to correctly retarget and relabel tags when executing rewrite operations across different execution backends, ensuring logs and metrics reflect the actual backend in use.
  • Tests

    • Added comprehensive test coverage for cross-backend tag relabeling including both successful relabeling and no-op verification scenarios.

Review Change Stack

The Tag* / TagPrefix* constants in rewriter_tags.go are VAAPI-canonical
(inlineass-vaapi, opencl-tonemap->vaapi, bitmap-inlineass-vaapi, etc.) because
VAAPI was the first backend implemented. After a cross-backend reshape onto
NVENC the rewriter executes the CUDA branch (inlineass(CUDA), tonemap_cuda),
but the emitted tags still showed vaapi-canonical names — so the 320/320 nvenc
qa_matrix pass logged things like
  hw-decode:filter:opencl-tonemap->vaapi:inlineass-vaapi
even though the actual filter graph was tonemap_cuda + inlineass(CUDA). The
output worked; the tags misrepresented the executed graph.

Fix: relabelCrossBackendTags() runs on res.Changes before the log emission.
Detects `cross-backend:*->X` and rewrites VAAPI-named substrings to the X
target's names (currently only nvenc mapping). Same-backend rewrites and
unknown target backends are no-ops.

Closes #113.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

📝 Walkthrough

Walkthrough

Adds relabelCrossBackendTags helper to post-process filter tags during cross-backend rewrite. Extracts target backend from cross-backend:*->X markers and rewrites VAAPI-canonical substrings to CUDA/NVENC equivalents. Integrated into handleTask rewriter path with comprehensive unit tests.

Changes

Cross-backend tag relabeling for NVENC rewrite

Layer / File(s) Summary
Relabeling logic and test coverage
worker/agent/rewriter_tags.go, worker/agent/rewriter_tags_relabel_test.go
relabelCrossBackendTags extracts target from cross-backend:*->X tags and performs ordered string rewrites for supported targets (nvenc), replacing VAAPI-canonical substrings (e.g., inlineass-vaapi, tonemap_vaapi) with CUDA equivalents. No-ops when marker missing, target empty, or target is canonical vaapi. Tests cover NVENC relabeling with multiple tag patterns and no-op cases (no marker, canonical target, unknown target).
Integration with rewriter pipeline
worker/agent/main.go
handleTask calls relabelCrossBackendTags(res.Changes) post-rewrite to ensure tags match the executed backend before logging and emission.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • Varashi/scaleplex#81: Renames NVIDIA dialect from "nvidia" to canonical "nvenc" (with "nvidia" alias), aligning the target backend value recognized by relabelCrossBackendTags.
  • Varashi/scaleplex#83: Implements reshapeForeignHWArgv that emits cross-backend:<src>-><dst> change tags consumed by this PR's relabeling logic.
  • Varashi/scaleplex#89: Adds cross-backend rewriter behavior and TagReplaceForeignInitHWDevice tag emission; this PR post-processes those tags to match execution context.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title accurately summarizes the main change: relabeling rewriter tags post-reshape to reflect actual executed backend, directly addressing issue #113.
Linked Issues check ✅ Passed Changes fully satisfy #113 requirements: detects cross-backend markers, rewrites VAAPI filter tags to target backend (NVENC/CUDA), calls relabel in tag emission path, includes comprehensive unit tests.
Out of Scope Changes check ✅ Passed All changes scoped to issue #113: relabelCrossBackendTags function, invocation in handleTask, and tests. No unrelated modifications or scope creep.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies"


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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 `@worker/agent/rewriter_tags_relabel_test.go`:
- Around line 52-62: Add a new table-driven test entry in
rewriter_tags_relabel_test.go that verifies relabeling of OpenCL tonemap to
VAAPI: include an input tag "filter:tonemap_opencl" (and any companion tags
needed by the existing table shape) and assert the expected output contains
"filter:tonemap_vaapi" and "force-output-format-vaapi" (or document an explicit
no-op if that's the intended behavior); place it alongside the existing cases
(e.g., near the "SW-tonemap normalized tag" case) so the test enforces the
contract for the OpenCL→VAAPI relabeling path.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 70a54d76-efb6-4d36-83a8-5318570cc932

📥 Commits

Reviewing files that changed from the base of the PR and between 71ce3ff and 2d9560e.

📒 Files selected for processing (3)
  • worker/agent/main.go
  • worker/agent/rewriter_tags.go
  • worker/agent/rewriter_tags_relabel_test.go

Comment on lines +52 to +62
{
name: "SW-tonemap normalized tag",
in: []string{
"cross-backend:vaapi->nvenc",
"filter:tonemap_opencl-normalized",
},
want: []string{
"cross-backend:vaapi->nvenc",
"filter:tonemap_cuda-normalized",
},
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add a case for filter:tonemap_opencl->tonemap_vaapi / force-output-format-vaapi.

Tied to the rep coverage gap above. If those tags reach the nvenc path, a row asserting they relabel (or a documented no-op) locks the contract.

🤖 Prompt for 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.

In `@worker/agent/rewriter_tags_relabel_test.go` around lines 52 - 62, Add a new
table-driven test entry in rewriter_tags_relabel_test.go that verifies
relabeling of OpenCL tonemap to VAAPI: include an input tag
"filter:tonemap_opencl" (and any companion tags needed by the existing table
shape) and assert the expected output contains "filter:tonemap_vaapi" and
"force-output-format-vaapi" (or document an explicit no-op if that's the
intended behavior); place it alongside the existing cases (e.g., near the
"SW-tonemap normalized tag" case) so the test enforces the contract for the
OpenCL→VAAPI relabeling path.

@Varashi Varashi merged commit 14b2757 into main May 30, 2026
3 checks passed
@Varashi Varashi deleted the fix/113-relabel-cross-backend-tags branch May 30, 2026 05:26
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.

qa_matrix tags mislabel cross-backend nvenc sub-burn as vaapi

1 participant