Swapable cleaner/raw diffs for Notebooks

Comments on the Commit diff are shared between the
raw and rendered version. Allows per-file toggling.

Upgrades ipynb to 0.4.2, fixing issues with unknown
output types in notebooks.

Commit https://gitlab.com/gitlab-org/gitlab/-/commit/71a1a01
introduced improvements on performance for loading MRs
by saving the diffs on the database, but didn't account for the possi-
bility of renderable diffs. For now, we are supressing this optimization
only for renderable diffs (ipynb files only).

Changelog: added

Gemfile

@@ -534,4 +534,4 @@ gem 'ipaddress', '~> 0.8.3'
 
 gem 'parslet', '~> 1.8'
 
-gem 'ipynbdiff', '0.3.8'
+gem 'ipynbdiff', '0.4.2'

Gemfile.lock

@@ -648,9 +648,9 @@ GEM
     invisible_captcha (1.1.0)
       rails (>= 4.2)
     ipaddress (0.8.3)
-    ipynbdiff (0.3.8)
-      diffy (= 3.3.0)
-      json (= 2.5.1)
+    ipynbdiff (0.4.2)
+      diffy (~> 3.3)
+      json (~> 2.5, >= 2.5.1)
     jaeger-client (1.1.0)
       opentracing (~> 0.3)
       thrift
@@ -1507,7 +1507,7 @@ DEPENDENCIES
   icalendar
   invisible_captcha (~> 1.1.0)
   ipaddress (~> 0.8.3)
-  ipynbdiff (= 0.3.8)
+  ipynbdiff (= 0.4.2)
   jira-ruby (~> 2.1.4)
   js_regex (~> 3.7)
   json (~> 2.5.1)

app/assets/javascripts/diff.js

 import $ from 'jquery';
+import { merge } from 'lodash';
 import createFlash from '~/flash';
 import axios from '~/lib/utils/axios_utils';
 import { __ } from '~/locale';
@@ -41,6 +42,8 @@ export default class Diff {
     }
 
     this.openAnchoredDiff();
+
+    this.prepareRenderedDiff();
   }
 
   handleClickUnfold(e) {
@@ -150,4 +153,43 @@ export default class Diff {
         .addClass('hll');
     }
   }
+
+  prepareRenderedDiff() {
+    const $elements = $('[data-diff-toggle-entity]');
+
+    if ($elements.length === 0) return;
+
+    const diff = this;
+
+    const elements = $elements.toArray().map(this.formatElementToObject).reduce(merge);
+
+    Object.values(elements).forEach((e) => {
+      e.toShowBtn.onclick = () => diff.showOneHideAnother('rendered', e); // eslint-disable-line no-param-reassign
+      e.toHideBtn.onclick = () => diff.showOneHideAnother('raw', e); // eslint-disable-line no-param-reassign
+
+      diff.showOneHideAnother('raw', e);
+    });
+  }
+
+  formatElementToObject = (element) => {
+    const key = element.attributes['data-file-hash'].value;
+    const name = element.attributes['data-diff-toggle-entity'].value;
+
+    return { [key]: { [name]: element } };
+  };
+
+  showOneHideAnother = (mode, elements) => {
+    let { toShowBtn, toHideBtn, toShow, toHide } = elements;
+
+    if (mode === 'raw') {
+      [toShowBtn, toHideBtn] = [toHideBtn, toShowBtn];
+      [toShow, toHide] = [toHide, toShow];
+    }
+
+    toShowBtn.classList.add('selected');
+    toHideBtn.classList.remove('selected');
+
+    toHide.classList.add('hidden');
+    toShow.classList.remove('hidden');
+  };
 }

app/controllers/projects/commit_controller.rb

@@ -164,6 +164,7 @@ class Projects::CommitController < Projects::ApplicationController
 
     opts = diff_options
     opts[:ignore_whitespace_change] = true if params[:format] == 'diff'
+    opts[:use_extra_viewer_as_main] = false
 
     @diffs = commit.diffs(opts)
     @notes_count = commit.notes.count

app/helpers/commits_helper.rb

@@ -182,6 +182,19 @@ module CommitsHelper
     project_commit_path(project, DEFAULT_SHA).sub("/#{DEFAULT_SHA}", '/$COMMIT_SHA')
   end
 
+  def diff_mode_swap_button(mode, file_hash)
+    icon = mode == 'raw' ? 'doc-code' : 'doc-text'
+    entity = mode == 'raw' ? 'toHideBtn' : 'toShowBtn'
+    title = "Display #{mode} diff"
+
+    link_to("##{mode}-diff-#{file_hash}",
+            class: "btn gl-button btn-default btn-file-option has-tooltip btn-show-#{mode}-diff",
+            title: title,
+            data: { file_hash: file_hash, diff_toggle_entity: entity }) do
+      sprite_icon(icon)
+    end
+  end
+
   protected
 
   # Private: Returns a link to a person. If the person has a matching user and

app/helpers/diff_helper.rb

@@ -28,7 +28,7 @@ module DiffHelper
   end
 
   def diff_options
-    options = { ignore_whitespace_change: hide_whitespace?, expanded: diffs_expanded? }
+    options = { ignore_whitespace_change: hide_whitespace?, expanded: diffs_expanded?, use_extra_viewer_as_main: true }
 
     if action_name == 'diff_for_path'
       options[:expanded] = true
@@ -74,7 +74,7 @@ module DiffHelper
   end
 
   def diff_link_number(line_type, match, text)
-    line_type == match ? " " : text
+    line_type == match || text == 0 ? " " : text
   end
 
   def parallel_diff_discussions(left, right, diff_file)

app/models/blobs/notebook.rb

+# frozen_string_literal: true
+
+module Blobs
+  class Notebook < ::Blob
+    attr_reader :data
+
+    def initialize(blob, data)
+      super(blob.__getobj__, blob.container)
+      @data = data
+    end
+  end
+end

app/models/diff_note.rb

@@ -145,7 +145,7 @@ class DiffNote < Note
   end
 
   def fetch_diff_file
-    return note_diff_file.raw_diff_file if note_diff_file
+    return note_diff_file.raw_diff_file if note_diff_file && !note_diff_file.raw_diff_file.has_renderable?
 
     if created_at_diff?(noteable.diff_refs)
       # We're able to use the already persisted diffs (Postgres) if we're

app/presenters/blob_presenter.rb

 # frozen_string_literal: true
-require 'ipynbdiff'
 
 class BlobPresenter < Gitlab::View::Presenter::Delegated
   include ApplicationHelper

app/presenters/blobs/notebook_presenter.rb

+# frozen_string_literal: true
+
+module Blobs
+  class NotebookPresenter < ::BlobPresenter
+    def gitattr_language
+      'md'
+    end
+  end
+end

app/views/projects/diffs/_content.html.haml

 - diff_file = local_assigns.fetch(:diff_file, nil)
+- file_hash = hexdigest(diff_file.file_path)
+
 .diff-content
+  - if diff_file.has_renderable?
+    %div{ id: "#raw-diff-#{file_hash}", data: { file_hash: file_hash, diff_toggle_entity: 'toHide' } }
       = render 'projects/diffs/viewer', viewer: diff_file.viewer
+    %div{ id: "#rendered-diff-#{file_hash}", data: { file_hash: file_hash, diff_toggle_entity: 'toShow' } }
+      = render 'projects/diffs/viewer', viewer: diff_file.rendered.viewer
+  - else
+    = render 'projects/diffs/viewer', viewer: diff_file.viewer
+

app/views/projects/diffs/_file.html.haml

@@ -25,6 +25,11 @@
             = edit_blob_button(@merge_request.source_project, @merge_request.source_branch, diff_file.new_path,
                              blob: diff_file.blob, link_opts: link_opts)
 
+        - if diff_file.has_renderable?
+          .btn-group.gl-ml-3
+            = diff_mode_swap_button('raw', file_hash)
+            = diff_mode_swap_button('rendered', file_hash)
+
         - if image_diff && image_replaced
           = view_file_button(diff_file.old_content_sha, diff_file.old_path, project, replaced: true)
 

app/views/projects/diffs/_line.html.haml

@@ -3,7 +3,7 @@
 - plain = local_assigns.fetch(:plain, false)
 - discussions = local_assigns.fetch(:discussions, nil)
 - line_code = diff_file.line_code(line)
-- if discussions && line.discussable?
+- if discussions
   - line_discussions = discussions[line_code]
 
 %tr.line_holder{ class: line.type, id: (line_code unless plain) }
@@ -15,10 +15,11 @@
     %td.new_line.diff-line-num
     %td.line_content.match= line.text
   - else
-    %td.old_line.diff-line-num{ class: [line.type, ("js-avatar-container" if !plain)], data: { linenumber: line.old_pos } }
+    %td.old_line.diff-line-num{ class: [line.type, ("js-avatar-container" unless plain)], data: { linenumber: line.old_pos } }
       - if plain
         = diff_link_number(line.type, "new", line.old_pos)
       - else
+        - if line.discussable?
           = add_diff_note_button(line_code, diff_file.position(line), line.type)
         %a{ href: "##{line_code}", data: { linenumber: diff_link_number(line.type, "new", line.old_pos) } }
 

config/feature_flags/development/jupyter_clean_diffs.yml → config/feature_flags/development/rendered_diffs_viewer.yml

 ---
-name: jupyter_clean_diffs
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/71477
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/343433
-milestone: '14.5'
+name: rendered_diffs_viewer
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75500
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/352831
+milestone: '14.9'
 type: development
 group: group::incubation
-default_enabled: true
+default_enabled: false

lib/gitlab/diff/custom_diff.rb

@@ -16,10 +16,12 @@ module Gitlab
         end
 
         def transformed_diff(before, after)
+          Gitlab::AppLogger.info({ message: 'IPYNB_DIFF_GENERATED' })
+
           transformed_diff = IpynbDiff.diff(before, after,
-                                            diff_opts: { context: 5, include_diff_info: true },
-                                            transform_options: { cell_decorator: :percent },
-                                            raise_if_invalid_notebook: true)
+                         raise_if_invalid_nb: true,
+                         diffy_opts: { include_diff_info: true }).to_s(:text)
+
           strip_diff_frontmatter(transformed_diff)
         end
 
@@ -29,9 +31,7 @@ module Gitlab
 
         def transformed_blob_data(blob)
           if transformed_for_diff?(blob)
-            IpynbDiff.transform(blob.data,
-                                raise_errors: true,
-                                options: { include_metadata: false, cell_decorator: :percent })
+            IpynbDiff.transform(blob.data, raise_errors: true, include_frontmatter: false)
           end
         end
 

lib/gitlab/diff/file.rb

@@ -44,11 +44,15 @@ module Gitlab
         new_blob_lazy
         old_blob_lazy
 
-        diff.diff = Gitlab::Diff::CustomDiff.preprocess_before_diff(diff.new_path, old_blob_lazy, new_blob_lazy) || diff.diff if use_custom_diff?
+        diff.diff = Gitlab::Diff::CustomDiff.preprocess_before_diff(diff.new_path, old_blob_lazy, new_blob_lazy) || diff.diff unless use_renderable_diff?
       end
 
-      def use_custom_diff?
-        strong_memoize(:_custom_diff_enabled) { Feature.enabled?(:jupyter_clean_diffs, repository.project, default_enabled: true) }
+      def use_renderable_diff?
+        strong_memoize(:_renderable_diff_enabled) { Feature.enabled?(:rendered_diffs_viewer, repository.project, default_enabled: :yaml) }
+      end
+
+      def has_renderable?
+        rendered&.has_renderable?
       end
 
       def position(position_marker, position_type: :text)
@@ -370,6 +374,12 @@ module Gitlab
         lines.none? { |line| line.type.to_s == 'match' }
       end
 
+      def rendered
+        return unless use_renderable_diff? && ipynb?
+
+        strong_memoize(:rendered) { Rendered::Notebook::DiffFile.new(self) }
+      end
+
       private
 
       def diffable_by_attribute?
@@ -399,6 +409,10 @@ module Gitlab
         new_file? || deleted_file? || content_changed?
       end
 
+      def ipynb?
+        modified_file? && file_path.ends_with?('.ipynb')
+      end
+
       # We can't use Object#try because Blob doesn't inherit from Object, but
       # from BasicObject (via SimpleDelegator).
       def try_blobs(meth)

lib/gitlab/diff/file_collection/base.rb

@@ -11,7 +11,7 @@ module Gitlab
         delegate :count, :size, :real_size, to: :raw_diff_files
 
         def self.default_options
-          ::Commit.max_diff_options.merge(ignore_whitespace_change: false, expanded: false, include_stats: true)
+          ::Commit.max_diff_options.merge(ignore_whitespace_change: false, expanded: false, include_stats: true, use_extra_viewer_as_main: false)
         end
 
         def initialize(diffable, project:, diff_options: nil, diff_refs: nil, fallback_diff_refs: nil)
@@ -25,6 +25,7 @@ module Gitlab
           @diff_refs = diff_refs
           @fallback_diff_refs = fallback_diff_refs
           @repository = project.repository
+          @use_extra_viewer_as_main = diff_options.delete(:use_extra_viewer_as_main)
         end
 
         def diffs
@@ -120,11 +121,17 @@ module Gitlab
 
           stats = diff_stats_collection&.find_by_path(diff.new_path)
 
-          Gitlab::Diff::File.new(diff,
+          diff_file = Gitlab::Diff::File.new(diff,
                                  repository: project.repository,
                                  diff_refs: diff_refs,
                                  fallback_diff_refs: fallback_diff_refs,
                                  stats: stats)
+
+          if @use_extra_viewer_as_main && diff_file.has_renderable?
+            diff_file.rendered
+          else
+            diff_file
+          end
         end
 
         def sort_diffs(diffs)

lib/gitlab/diff/file_collection/merge_request_diff_base.rb

@@ -15,7 +15,8 @@ module Gitlab
                 project: merge_request_diff.project,
                 diff_options: diff_options,
                 diff_refs: merge_request_diff.diff_refs,
-            fallback_diff_refs: merge_request_diff.fallback_diff_refs)
+                fallback_diff_refs: merge_request_diff.fallback_diff_refs
+          )
         end
 
         def diff_files(sorted: false)

lib/gitlab/diff/line.rb

@@ -8,9 +8,9 @@ module Gitlab
       #
       SERIALIZE_KEYS = %i(line_code rich_text text type index old_pos new_pos).freeze
 
-      attr_reader :line_code, :marker_ranges
-      attr_writer :text, :rich_text
-      attr_accessor :index, :type, :old_pos, :new_pos
+      attr_reader :marker_ranges
+      attr_writer :text, :rich_text, :discussable
+      attr_accessor :index, :type, :old_pos, :new_pos, :line_code
 
       def initialize(text, type, index, old_pos, new_pos, parent_file: nil, line_code: nil, rich_text: nil)
         @text = text
@@ -26,6 +26,7 @@ module Gitlab
         @line_code = line_code || calculate_line_code
 
         @marker_ranges = []
+        @discussable = true
       end
 
       def self.init_from_hash(hash)
@@ -96,7 +97,7 @@ module Gitlab
       end
 
       def discussable?
-        !meta?
+        @discussable && !meta?
       end
 
       def suggestible?

lib/gitlab/diff/rendered/notebook/diff_file.rb

+# frozen_string_literal: true
+module Gitlab
+  module Diff
+    module Rendered
+      module Notebook
+        include Gitlab::Utils::StrongMemoize
+
+        class DiffFile < Gitlab::Diff::File
+          attr_reader :source_diff
+
+          delegate :repository, :diff_refs, :fallback_diff_refs, :unfolded, :unique_identifier,
+                   to: :source_diff
+
+          def initialize(diff_file)
+            @source_diff = diff_file
+          end
+
+          def old_blob
+            return unless notebook_diff
+
+            strong_memoize(:old_blob) { ::Blobs::Notebook.decorate(source_diff.old_blob, notebook_diff.from.as_text) }
+          end
+
+          def new_blob
+            return unless notebook_diff
+
+            strong_memoize(:new_blob) { ::Blobs::Notebook.decorate(source_diff.new_blob, notebook_diff.to.as_text) }
+          end
+
+          def diff
+            strong_memoize(:diff) { transformed_diff }
+          end
+
+          def has_renderable?
+            !notebook_diff.nil? && diff.diff.present?
+          end
+
+          def rendered
+            self
+          end
+
+          def highlighted_diff_lines
+            @highlighted_diff_lines ||= begin
+              removal_line_maps, addition_line_maps = compute_end_start_map
+              Gitlab::Diff::Highlight.new(self, repository: self.repository).highlight.map do |line|
+                mutate_line(line, addition_line_maps, removal_line_maps)
+              end
+            end
+          end
+
+          private
+
+          def notebook_diff
+            strong_memoize(:notebook_diff) do
+              Gitlab::AppLogger.info({ message: 'IPYNB_DIFF_GENERATED' })
+
+              IpynbDiff.diff(source_diff.old_blob&.data, source_diff.new_blob&.data,
+                             raise_if_invalid_nb: true,
+                             diffy_opts: { include_diff_info: true })
+            rescue IpynbDiff::InvalidNotebookError => e
+              Gitlab::ErrorTracking.log_exception(e)
+              nil
+            end
+          end
+
+          def transformed_diff
+            return unless notebook_diff
+
+            diff = source_diff.diff.clone
+            diff.diff = strip_diff_frontmatter(notebook_diff.to_s(:text))
+            diff
+          end
+
+          def strip_diff_frontmatter(diff_content)
+            diff_content.scan(/.*\n/)[2..]&.join('') if diff_content.present?
+          end
+
+          def transformed_line_to_source(transformed_line, transformed_blocks)
+            transformed_blocks.empty? ? 0 : ( transformed_blocks[transformed_line - 1][:source_line] || -1 ) + 1
+          end
+
+          def mutate_line(line, addition_line_maps, removal_line_maps)
+            line.new_pos = transformed_line_to_source(line.new_pos, notebook_diff.to.blocks)
+            line.old_pos = transformed_line_to_source(line.old_pos, notebook_diff.from.blocks)
+
+            line.old_pos = addition_line_maps[line.new_pos] if line.old_pos == 0 && line.new_pos != 0
+            line.new_pos = removal_line_maps[line.old_pos] if line.new_pos == 0 && line.old_pos != 0
+
+            # Lines that do not appear on the original diff should not be commentable
+
+            unless addition_line_maps[line.new_pos] || removal_line_maps[line.old_pos]
+              line.discussable = false
+            end
+
+            line.line_code = line_code(line)
+            line
+          end
+
+          def compute_end_start_map
+            # line_codes are used for assigning notes to diffs, and these depend on the line on the new version and the
+            # line that would have been that one in the previous version. However, since we do a transformation on the
+            # file, that map gets lost. To overcome this, we look at the original source lines and build two maps:
+            # - For additions, we look at the latest line change for that line and pick the old line for that id
+            # - For removals, we look at the first line in the old version, and pick the first line on the new version
+            #
+            #
+            # The caveat here is that we can't have notes on lines that are not a translation of a line in the source
+            # diff
+            #
+            # (gitlab/diff/file.rb:75)
+
+            removals = {}
+            additions = {}
+
+            source_diff.highlighted_diff_lines.each do |line|
+              removals[line.old_pos] = line.new_pos
+              additions[line.new_pos] = line.old_pos
+            end
+
+            [removals, additions]
+          end
+        end
+      end
+    end
+  end
+end

spec/features/projects/commits/multi_view_diff_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.shared_examples "no multiple viewers" do |commit_ref|
+  let(:ref) { commit_ref }
+
+  it "does not display multiple diff viewers" do
+    expect(page).not_to have_selector '[data-diff-toggle-entity]'
+  end
+end
+
+RSpec.describe 'Multiple view Diffs', :js do
+  let(:user) { create(:user) }
+  let(:project) { create(:project, :repository, visibility_level: Gitlab::VisibilityLevel::PUBLIC) }
+  let(:ref) { '5d6ed1503801ca9dc28e95eeb85a7cf863527aee' }
+  let(:path) { project_commit_path(project, ref) }
+  let(:feature_flag_on) { false }
+
+  before do
+    stub_feature_flags(rendered_diffs_viewer: feature_flag_on ? project : false)
+
+    visit path
+
+    wait_for_all_requests
+  end
+
+  context 'when :rendered_diffs_viewer is off' do
+    context 'and diff does not have ipynb' do
+      include_examples "no multiple viewers", 'ddd0f15ae83993f5cb66a927a28673882e99100b'
+    end
+
+    context 'and diff has ipynb' do
+      include_examples "no multiple viewers", '5d6ed1503801ca9dc28e95eeb85a7cf863527aee'
+
+      it 'shows the transformed diff' do
+        diff = page.find('.diff-file, .file-holder', match: :first)
+
+        expect(diff['innerHTML']).to include('%% Cell type:markdown id:0aac5da7-745c-4eda-847a-3d0d07a1bb9b tags:')
+      end
+    end
+  end
+
+  context 'when :rendered_diffs_viewer is on' do
+    let(:feature_flag_on) { true }
+
+    context 'and diff does not include ipynb' do
+      include_examples "no multiple viewers", 'ddd0f15ae83993f5cb66a927a28673882e99100b'
+    end
+
+    context 'and opening a diff with ipynb' do
+      context 'but the changes are not renderable' do
+        include_examples "no multiple viewers", 'a867a602d2220e5891b310c07d174fbe12122830'
+      end
+
+      it 'loads the rendered diff as hidden' do
+        diff = page.find('.diff-file, .file-holder', match: :first)
+
+        expect(diff).to have_selector '[data-diff-toggle-entity="toHide"]'
+        expect(diff).not_to have_selector '[data-diff-toggle-entity="toShow"]'
+
+        expect(classes_for_element(diff, 'toShow', visible: false)).to include('hidden')
+        expect(classes_for_element(diff, 'toHide')).not_to include('hidden')
+
+        expect(classes_for_element(diff, 'toHideBtn')).to include('selected')
+        expect(classes_for_element(diff, 'toShowBtn')).not_to include('selected')
+      end
+
+      it 'displays the rendered diff and hides after selection changes' do
+        diff = page.find('.diff-file, .file-holder', match: :first)
+        diff.find('[data-diff-toggle-entity="toShowBtn"]').click
+
+        expect(diff).to have_selector '[data-diff-toggle-entity="toShow"]'
+        expect(diff).not_to have_selector '[data-diff-toggle-entity="toHide"]'
+
+        expect(classes_for_element(diff, 'toHideBtn')).not_to include('selected')
+        expect(classes_for_element(diff, 'toShowBtn')).to include('selected')
+      end
+    end
+  end
+
+  def classes_for_element(node, data_diff_entity, visible: true)
+    node.find("[data-diff-toggle-entity=\"#{data_diff_entity}\"]", visible: visible)[:class]
+  end
+end

spec/helpers/commits_helper_spec.rb

@@ -86,6 +86,31 @@ RSpec.describe CommitsHelper do
     end
   end
 
+  describe '#diff_mode_swap_button' do
+    let(:keyword) { 'rendered' }
+    let(:node) { Nokogiri::HTML.parse(helper.diff_mode_swap_button(keyword, 'abc')).at_css('a') }
+
+    context 'for rendered' do
+      it 'renders the correct select-rendered button' do
+        expect(node[:title]).to eq('Display rendered diff')
+        expect(node['data-file-hash']).to eq('abc')
+        expect(node['data-diff-toggle-entity']).to eq('toShowBtn')
+        expect(node.xpath("//a/svg")[0]["data-testid"]).to eq('doc-text-icon')
+      end
+    end
+
+    context 'for raw' do
+      let(:keyword) { 'raw' }
+
+      it 'renders the correct select-raw button' do
+        expect(node[:title]).to eq('Display raw diff')
+        expect(node['data-file-hash']).to eq('abc')
+        expect(node['data-diff-toggle-entity']).to eq('toHideBtn')
+        expect(node.xpath("//a/svg")[0]["data-testid"]).to eq('doc-code-icon')
+      end
+    end
+  end
+
   describe '#commit_to_html' do
     let(:project) { create(:project, :repository) }
     let(:ref) { 'master' }

spec/lib/gitlab/diff/file_spec.rb

@@ -51,45 +51,29 @@ RSpec.describe Gitlab::Diff::File do
     project.commit(branch_name).diffs.diff_files.first
   end
 
-  describe 'initialize' do
-    context 'when file is ipynb with a change after transformation' do
+  describe '#has_renderable?' do
+    context 'file is ipynb' do
       let(:commit) { project.commit("532c837") }
-      let(:diff) { commit.raw_diffs.first }
-      let(:diff_file) { described_class.new(diff, diff_refs: commit.diff_refs, repository: project.repository) }
 
-      context 'and :jupyter_clean_diffs is enabled' do
-        before do
-          stub_feature_flags(jupyter_clean_diffs: true)
-        end
-
-        it 'recreates the diff by transforming the files' do
-          expect(diff_file.diff.diff).not_to include('cell_type')
+      it 'has renderable viewer' do
+        expect(diff_file.has_renderable?).to be_truthy
       end
     end
 
-      context 'but :jupyter_clean_diffs is disabled' do
-        before do
-          stub_feature_flags(jupyter_clean_diffs: false)
-        end
+    context 'file is not ipynb' do
+      let(:commit) { project.commit("d59c60028b053793cecfb4022de34602e1a9218e") }
 
-        it 'does not recreate the diff' do
-          expect(diff_file.diff.diff).to include('cell_type')
+      it 'does not have renderable viewer' do
+        expect(diff_file.has_renderable?).to be_falsey
       end
     end
   end
 
-    context 'when file is ipynb, but there only changes that are removed' do
-      let(:commit) { project.commit("2b5ef814") }
-      let(:diff) { commit.raw_diffs.first }
-      let(:diff_file) { described_class.new(diff, diff_refs: commit.diff_refs, repository: project.repository) }
-
-      before do
-        stub_feature_flags(jupyter_clean_diffs: true)
-      end
+  describe '#rendered' do
+    let(:commit) { project.commit("532c837") }
 
-      it 'does not recreate the diff' do
-        expect(diff_file.diff.diff).to include('execution_count')
-      end
+    it 'creates a NotebookDiffFile for rendering' do
+      expect(diff_file.rendered).to be_kind_of(Gitlab::Diff::Rendered::Notebook::DiffFile)
     end
   end
 

spec/lib/gitlab/diff/rendered/notebook/diff_file_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Diff::Rendered::Notebook::DiffFile do
+  include RepoHelpers
+
+  let(:project) { create(:project, :repository) }
+  let(:commit) { project.commit("5d6ed1503801ca9dc28e95eeb85a7cf863527aee") }
+  let(:diffs) { commit.raw_diffs.to_a }
+  let(:diff) { diffs.first }
+  let(:source) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: project.repository) }
+  let(:nb_file) { described_class.new(source) }
+
+  describe '#old_blob and #new_blob' do
+    context 'when file is changed' do
+      it 'transforms the old blob' do
+        expect(nb_file.old_blob.data).to include('%%')
+      end
+
+      it 'transforms the new blob' do
+        expect(nb_file.new_blob.data).to include('%%')
+      end
+    end
+
+    context 'when file is added' do
+      let(:diff) { diffs[1] }
+
+      it 'old_blob is empty' do
+        expect(nb_file.old_blob).to be_nil
+      end
+
+      it 'new_blob is transformed' do
+        expect(nb_file.new_blob.data).to include('%%')
+      end
+    end
+
+    context 'when file is removed' do
+      let(:diff) { diffs[2] }
+
+      it 'old_blob is transformed' do
+        expect(nb_file.old_blob.data).to include('%%')
+      end
+
+      it 'new_blob is empty' do
+        expect(nb_file.new_blob).to be_nil
+      end
+    end
+  end
+
+  describe '#diff' do
+    context 'for valid notebooks' do
+      it 'returns the transformed diff' do
+        expect(nb_file.diff.diff).to include('%%')
+      end
+    end
+
+    context 'for invalid notebooks' do
+      let(:commit) { project.commit("6d85bb693dddaee631ec0c2f697c52c62b93f6d3") }
+      let(:diff) { diffs[1] }
+
+      it 'returns nil' do
+        expect(nb_file.diff).to be_nil
+      end
+    end
+  end
+
+  describe '#has_renderable?' do
+    context 'notebook diff is empty' do
+      let(:commit) { project.commit("a867a602d2220e5891b310c07d174fbe12122830") }
+
+      it 'is false' do
+        expect(nb_file.has_renderable?).to be_falsey
+      end
+    end
+
+    context 'notebook is valid' do
+      it 'is true' do
+        expect(nb_file.has_renderable?).to be_truthy
+      end
+    end
+  end
+
+  describe '#highlighted_diff_lines?' do
+    context 'when line transformed line is not part of the diff' do
+      it 'line is not discussable' do
+        expect(nb_file.highlighted_diff_lines[0].discussable?).to be_falsey
+      end
+    end
+
+    context 'when line transformed line part of the diff' do
+      it 'line is not discussable' do
+        expect(nb_file.highlighted_diff_lines[12].discussable?).to be_truthy
+      end
+    end
+
+    context 'assigns the correct position' do
+      it 'computes de first line where the remove would appear' do
+        expect(nb_file.highlighted_diff_lines[0].old_pos).to eq(3)
+        expect(nb_file.highlighted_diff_lines[0].new_pos).to eq(3)
+
+        expect(nb_file.highlighted_diff_lines[12].new_pos).to eq(15)
+        expect(nb_file.highlighted_diff_lines[12].old_pos).to eq(18)
+      end
+    end
+  end
+end

spec/presenters/blobs/notebook_presenter_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Blobs::NotebookPresenter do
+  include RepoHelpers
+
+  let(:project) { create(:project, :repository) }
+  let(:repository) { project.repository }
+  let(:blob) { repository.blob_at('HEAD', 'files/ruby/regex.rb') }
+  let(:user) { project.first_owner }
+  let(:git_blob) { blob.__getobj__ }
+
+  subject(:presenter) { described_class.new(blob, current_user: user) }
+
+  it 'highlight receives markdown' do
+    expect(Gitlab::Highlight).to receive(:highlight).with('files/ruby/regex.rb', git_blob.data, plain: nil, language: 'md')
+
+    presenter.highlight
+  end
+end

spec/support/helpers/test_env.rb

@@ -54,7 +54,7 @@ module TestEnv
     'wip'                                => 'b9238ee',
     'csv'                                => '3dd0896',
     'v1.1.0'                             => 'b83d6e3',
-    'add-ipython-files'                  => '532c837',
+    'add-ipython-files'                  => 'a867a602',
     'add-pdf-file'                       => 'e774ebd',
     'squash-large-files'                 => '54cec52',
     'add-pdf-text-binary'                => '79faa7b',