Skip to content

feat(merge_requests): show original multiline discussion context - #595

Merged
sjungwon03 merged 2 commits into
devfrom
feat/594-multiline-context
Oct 4, 2026
Merged

sjungwon03 merged 2 commits into
devfrom
feat/594-multiline-context

Conversation

@sjungwon03

Copy link
Copy Markdown
Member

Summary

Original MR discussion context now resolves complete multiline text ranges and marks every inclusive covered line, alongside the existing single-line flow. Start/end line codes must match the exact original filename hash, raw counters, side metadata and any supplied visible coordinates. Same-side ranges exclude opposite-side rows; forward mixed-side ranges retain their unified-diff interval. Missing optional coordinates are resolved by the full code without inventing a legacy anchor.

Covered hunks must be complete and contiguous. Wrong, ambiguous, reversed, malformed or gapped spans remain unavailable without a newer-diff or single-line fallback. Shared viewer markers are accessible and independent of color. Existing explicit older-version pagination, account/resource isolation and comment drafts are retained in one responsive UI.

Closes #594

Validation

  • dart format .: 903 Dart files; no outstanding formatting changes.
  • flutter analyze --no-pub: no issues.
  • All 5,667 tests pass: app 3,164; design system 125; API 2,201; models 172; secure storage 5. Includes 72 new app/shared-viewer cases.
  • Test-first missing range state/viewer contracts, then reproduced optional-coordinate and oversized endpoint/hunk count failures before fixes.
  • Five locales × three widths × both themes, compact keyboard/doubled-text layout, resize/theme drafts and resource replacement.
  • Four synthetic captures generated and visually inspected. Locale delegates reproduce byte-for-byte with flutter gen-l10n.
  • No model/API changes; build_runner is not applicable.
  • Live GitLab/physical-device validation: not performed.

Checklist

  • Targets dev; issue linked; conventional signed-off commits.
  • Documentation and parity tracker updated; mobile/desktop and both themes checked.
  • Existing crypto version added as a direct app dependency; BSD-3-Clause license verified, notices updated, and full package license collected by Flutter. No new version or lockfile change.
  • No real credentials, private metadata or source in fixtures/captures.
  • Repository English policy followed; only locale resources/generated translations contain non-English text.

Screenshots

Synthetic widget-test captures at d279ad0119c02c23495ebb96ecada275d152af1e, using dummy same-path fixtures and SDK fonts. Renamed-path matching is covered by tests; the SDK-only capture font lacks the rename-arrow glyph.

Layout Light Dark
Mobile Mobile light Mobile dark
Desktop Desktop light Desktop dark

Notes for reviewers

This is read-only original context. No discussion creation or suggestion mutation is added. A range cannot cross omitted blob context, and reversed mixed-side endpoints are not reinterpreted. Full blob unfolding and multiline positioned thread creation remain separate work. Existing single-line original-context and shared viewer regressions pass.

See docs/mr-discussion-context.md, GitLab multiline fields and line-code identity. Upstream filename/counter semantics were inspected; implementation code was independently written.

Verify the full original range endpoint identity and every available intervening hunk so multiline review never highlights a guessed or newer location; retain accessible markers, drafts and session isolation.

Signed-off-by: sjungwon03 <sjungwon03@gmail.com>
@github-actions github-actions Bot added area:ui Screens and feature UI area:design-system packages/design_system area:docs AGENTS.md, .agents/, README and friends feat New feature labels Oct 4, 2026
Link the reviewed original multiline context and distinguish the remaining positioned range creation work.

Signed-off-by: sjungwon03 <sjungwon03@gmail.com>

@sjungwon03-ai sjungwon03-ai left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review of commit 9cb3e76: Reviewed the original-version/path boundary, complete range endpoint validation, filename hash/raw counter identity, optional-coordinate handling, exact visible side membership, uniqueness and direction checks, complete contiguous hunk requirements, shared identity-based markers and localized range presentation. Same-side ranges exclude opposite-side rows; forward mixed ranges preserve unified order. Unsupported or gapped ranges never fall back to the legacy single-line anchor or newer files. Existing explicit version pagination, session/resource isolation and draft behavior remain covered. The read flow adds no mutation.

No blocking findings. Local validation passed 5,667 tests across five packages, including 72 new app/shared-viewer cases; 903 Dart files are formatted and analysis is clean. Locale delegates regenerate byte-for-byte. Test-first evidence includes absent range state/viewer support, optional-coordinate failures and oversized endpoint/hunk counter failures before fixes. Five locales, three widths and both themes, compact keyboard/doubled text, lifecycle/resize cases and four visually inspected synthetic captures cover presentation. The existing crypto version is used directly by the app after BSD-3-Clause license verification and notice updates, with no new package version. No live GitLab/device validation is claimed. Omitted blob context and reversed mixed-side endpoints remain unavailable; multiline positioned creation is separate. Final-head CI, current-head identity and review-thread resolution are verified before approval.

@sjungwon03
sjungwon03 merged commit a69d661 into dev Oct 4, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:design-system packages/design_system area:docs AGENTS.md, .agents/, README and friends area:ui Screens and feature UI feat New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show exact original multiline discussion context

2 participants