fix(diff): preserve diff side for review comment locations

[chaojixinren]

Description

Fixes #1486.

When existing_code matches deleted code, the resolver already finds the correct old-file line number. However, LlmComment did not preserve whether the coordinate belonged to the old or new side of the diff. Downstream consumers therefore interpreted an old-file line as a new-file line and could attach the comment to unrelated code.

This change adds an optional side field to review comments:

• RIGHT identifies coordinates in the new file.
• LEFT identifies coordinates in the old file.
• Comments without side retain the legacy RIGHT-side behavior.

The side metadata is carried through:

• JSON and text output
• SARIF locations
• GitHub PR review comments
• Viewer session storage and rendering
• VS Code comment parsing and anchoring
• IDEA comment parsing and anchoring

Deleted-side SARIF findings retain their location metadata but do not emit replacements, because a deleted region cannot safely describe a replacement in the current file. In Workspace mode, LEFT-side comments remain viewable but cannot be applied to the current file.

The follow-up changes also localize the LEFT label and old-side apply warning, remove nested ternaries from the VS Code anchor resolver, and centralize IDEA side constants.

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring (no functional changes)
• Documentation update
• CI / Build / Tooling

How Has This Been Tested?

• make test passes locally with -race -count=1
• VS Code Jest: 96 tests passed
• VS Code lint passed
• VS Code TypeScript compilation passed
• Frontend Jest: 17 tests passed
• Frontend TypeScript typecheck passed
• GitHub Action comment tests passed
• git diff --check passed
• License and English-only checks passed

IDEA Gradle tests were not run because the local environment does not have a Java runtime.

Checklist

• My code follows the project's coding style (go fmt, go vet)
• I have performed a self-review of my code
• I have added tests that prove my fix is effective or my feature works
• New and existing unit tests pass locally with my changes
• I have updated the documentation accordingly
• I have signed the CLA
• I disclosed the AI/LLM assistance used below and reviewed the generated changes

AI Assistance Disclosure

This PR was implemented with OpenAI Codex (GPT-6) in Codex Desktop. I reviewed the generated code, tests, and documentation and understand the implementation.

Related Issues

Fixes #1486

[chaojixinren]

Design Rationale

A schema-compatible alternative was considered in PR #1518: when a comment matches deleted code, it resets start_line and end_line to 0.

That approach prevents comments from being attached to unrelated new-file lines, but it also makes deleted-code findings unanchored. They can only appear in the summary and lose the precise location in the old file.

This PR intentionally carries an explicit side field instead. GitHub Review API supports LEFT and RIGHT coordinates, so deleted-code comments can retain their exact old-file location while preserving safe behavior across all consumers.

[github-actions]

🔍 OpenCodeReview found 2 issue(s) in this PR.

• ✅ Successfully posted inline: 1 comment(s)
• ❌ Failed to post inline: 1 comment(s)

---

📄 extensions/idea/src/main/kotlin/com/alibaba/opencodereview/idea/providers/CommentAnchor.kt (L174-L178)

⚠️ GitHub could not post this as an inline comment: Lines 174-178 could not be resolved (outside PR diff hunks)

Minor inconsistency with the VSCode implementation: in workspace mode, the VSCode plugin checks comment.side === 'LEFT' before reading the workspace file and returns sidebar-only immediately. Here, the workspace file is read first (line 175) and only then rejected inside mountableOrUnresolved. Consider adding an early return for LEFT-side comments in workspace mode to avoid the unnecessary I/O:

if (ctx.mode == ReviewMode.WORKSPACE) {
    if (comment.side == COMMENT_SIDE_LEFT) {
        return CommentAnchorResult.SidebarOnly(SidebarOnlyReason.UNRESOLVED)
    }
    val content = git.readWorkspaceFile(comment.path)
        ?: return CommentAnchorResult.SidebarOnly(SidebarOnlyReason.MISSING_FILE)
    return mountableOrUnresolved(comment, content, AnchorSide.WORKSPACE, locale)
}

This also makes the guard in mountableOrUnresolved redundant for the workspace path (though keeping it there as a safety net is fine).

[Qiyuanqiii]

@NanaseInori review

[NanaseInori]

I reviewed the current head (8a944f0). The core resolver change makes sense, and the GitHub / Viewer / VS Code / IDEA / SARIF propagation is generally coherent. I do see one cross-integration blocker, though.

The new side field is necessary precisely because a positive start_line / end_line is no longer sufficient to tell consumers which file version the coordinate belongs to. However, several first-party publishing integrations still ignore side and continue treating every positive line as a new/current-side coordinate.

For example, examples/gitlab_ci/post_review.py still turns every positive end_line into:

"new_line": end_line

without checking comment["side"].

A LEFT finding therefore sends an old-file line number as a GitLab new_line, recreating the same class of misattachment that #1486 is fixing for GitHub.

examples/gitflic_ci/post_review.py has the same assumption even more explicitly: its current documentation/comment says that ocr review only reports new-side positions, and the publisher uses end_line as newLine before deriving oldLine from it. That assumption becomes false once this PR starts emitting old-side coordinates.

examples/gerrit_ci/post_review.py likewise uses end_line directly as the current patch-set line without considering side.

I think these consumers need to be updated together with the output-schema change. Where a target platform supports an old-side coordinate, LEFT should be mapped correctly. Where it does not, the safe behavior is to fold the finding into the summary/non-inline path instead of posting an old-file line as a current-file line. Any incremental/deduplication logic that compares locations should also account for the side where applicable.

Please add LEFT regression coverage for the affected integration scripts as well.

There is also a GitHub multi-line edge case that I think should be pinned before considering the side model complete.

resolveFromHunk constructs the old side from context + deleted lines. Therefore an existing_code span such as:

 keepBefore()
-legacyCall()
 keepAfter()

can resolve across a context line and a deleted line. The current model stores one Side for the whole resolved range, and the GitHub publisher then copies it to both start_side and side.

That means a mixed context/deletion range is represented as start_side=LEFT, side=LEFT even though the two endpoints do not necessarily have the same diff-side semantics. The current GitHub regression constructs a synthetic LEFT range directly, so it does not exercise this case from a real resolver result.

Please add a resolver-to-publisher regression for a multi-line old-side match containing context + deleted lines and either preserve the endpoint sides needed to represent it correctly, normalize the range to a safely representable deleted-side location, or degrade it rather than assuming one Side always describes both endpoints.

A documentation follow-up is also needed: this PR changes a public JSON field but only updates the English docs, and both open-code-review/SKILL.md copies still describe comment locations only in terms of start_line / end_line.

Other than these location-propagation issues, the core implementation looks solid to me. All current CI, CodeQL, IDEA, VS Code, frontend, Pages, translation-sync, and contract checks are green.

Requesting changes for the remaining side-unaware publishers and the mixed-side multi-line location case.

[chaojixinren]

@Qiyuanqiii Thanks for the detailed review. I addressed the requested issues in the latest commit.

Changes made:

• Updated the GitLab, Gerrit, and GitFlic publishers to preserve LEFT/old-side semantics.
• Added side-aware incremental deduplication and GitLab old-side diff inventory handling.
• Fixed GitLab history parsing for positions where the unused coordinate is present with a null value.
• Fixed incremental deduplication for renamed files by matching comments using new_path.
• Disabled GitLab’s directly applicable suggestion block for LEFT comments and render the suggested code as plain text instead.
• Added the old-side marker to GitLab dry-run output.
• Kept the GitLab dry-run diff inventory shape consistent with the real poster.
• Preserved the IDEA distinction between MISSING_FILE and UNRESOLVED, including deleted workspace files, while avoiding unnecessary workspace reads for LEFT comments.
• Added resolver, publisher, and integration regression coverage for old-side single-line and multi-line comments, including context-plus-deleted ranges.
• Updated the GitFlic documentation, localized documentation, and both open-code-review skill files to describe side.

Validation completed:

• make check
• GitLab publisher tests: 164 passed
• Gerrit publisher tests: 62 passed
• GitFlic publisher tests: 15 passed
• GitHub Actions post-review tests passed
• Go diff tests passed
• Python compilation and git diff --check passed