[YLlllllllllll]

Problem

The legacy packed-sequence tail detector feeds a boolean mask to cumprod. On the tested Ascend runtime this raises aclnnCumprod's unsupported DT_BOOL error, so DPO and other callers can fail before completing a training step.

Convert the zero/one mask to an integer before calculating its prefix product. The resulting segment lengths and explicit-tail path retain their existing semantics. The legacy path still cannot infer whether a trailing one-token segment is real data or padding without authoritative metadata; this PR does not change that behavior.

Validation

• CPU: 17 passed and 6 NPU cases skipped for the focused tail-segment and attention-metadata tests. The checked-in NPU cases are selected by the existing tests/data suite and require an actual NPU on an NPU CI runner.
• Compared the original and fixed functions on 21,844 finite segment/tail combinations with identical outputs.
• Ascend 910B2C: the standalone exact head 73cb97c489d1f244d107d546ddf3ced9ceee8de0 passed all 6/6 native boundary cases, no skips. A separate integration source 77da537bbf733e970bd1592c4e411fc19acccfc9 also passed four multi-rank DPO gradient cases. Public NPU CI approval and full-model validation remain separate.
• make quality, focused device-API check, and an independent code review passed. Simulated CI inventories verified NPU cases request a device and an NPU-designated runner without a working NPU fails rather than silently skipping.

Summary by CodeRabbit

• Tests
  • Added CPU and Ascend NPU test coverage for sequence-length processing across six input and tail-padding cases.
  • Tests verify expected results and int32 output on both devices. NPU test eligibility follows configured CI capabilities, or requires NPU support and availability when no capability setting is provided.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

This change adds CPU and Ascend tests for valid_seqlens_from_cu_seqlens, including six input cases, expected values, and int32 output checks. It also tests NPU test eligibility. The reported utility line is unchanged.

Changes: Sequence-length conversion tests

Layer / File(s)	Summary
Check conversion on CPU and Ascend tests/data/test_valid_seqlens_devices.py, veomni/utils/seqlen_pos_transform_utils.py	Tests check six input cases, expected values, and int32 output on CPU and Ascend. They also check NPU test eligibility. The reported padding calculation line is unchanged.

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

Change: Bug fix

Possibly related issues

• [Ascend]: 2026 Q4 NPU Roadmap #1405: The issue includes NPU regression coverage for packed-sequence boundaries. These tests cover related sequence-length cases on Ascend.

Suggested reviewers: coach257

Merge Risk: Minimal · up to d9b26

The change adds focused device coverage without introducing a verified behavioral risk.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title "[data] fix: support Ascend tail-segment cumprod" matches the main change. The PR converts the legacy packed-sequence zero/one mask to an integer before calling cumprod, because the tested A…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title "[data] fix: support Ascend tail-segment cumprod" matches the main change. The PR converts the legacy packed-sequence zero/one mask to an integer before calling cumprod, because the tested Ascend runtime rejects DT_BOOL for aclnnCumprod. The title names the Ascend target, the tail-segment logic, and the cumprod operation. The summary also covers new CPU and Ascend NPU tests, but the title does not need to list them.

Full details: Docstring Coverage

Explanation

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 2 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Recent review info

Run configuration

• Source mode: Auto
• Repository: ByteDance-Seed/VeOmni
• Branch: main
• File: .coderabbit.yaml
• Commit: 6e8433a
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 8af141fb-4089-4cce-b701-db5946ac8d72

Commits

Reviewing files that changed from the base of the PR and between 73cb97c and d9b2686.

Files selected for processing: 1

• veomni/utils/seqlen_pos_transform_utils.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

[YLlllllllllll]

Native Ascend 910B2C validation on exact head 73cb97c489d1f244d107d546ddf3ced9ceee8de0: test_valid_seqlens_ascend completed 6/6 with no skips, including explicit zero tail, single-token segments and empty input. This covers the native segment-length operation, not whole-model training. Public NPU CI still awaits approval.

[YLlllllllllll]

Upstream main now converts the reversed tail-segment mask to int32 before cumprod. The current head d9b26869dadfbf0b5fd8f7686a42a92f2b394fa8 aligns the helper with that upstream implementation and keeps the existing CPU/Ascend test cases. This resolves the content conflict without removing the exact-tail-padding behavior.

CPU tail-segment and padding regressions passed. The earlier 6/6 Ascend result was obtained on 73cb97c4 with the equivalent cast-before-flip ordering; the updated head's native Ascend rerun is pending.