Merge does not work for some notebooks

[krassowski]

This was previously reported in #550 (comment). This is the full traceback:

Uncaught (in promise) Error: Currently not able to handle decisions on variable "id"
    n exceptions.ts:19
    splitPatch cell.ts:550
    applyCellLevelDecision cell.ts:500
    processDecisions cell.ts:366
    m cell.ts:132
    buildCellList notebook.ts:235
    <anonymous> notebook.ts:90
    t merge.ts:76
    B merge.ts:166

Both versions 3.2 and 4.0 alpha are affected. Manually stripping cell IDs solves the problem for both versions. This can be reproduced by running:

nbmerge-web examples/example8/center.ipynb examples/example8/left.ipynb examples/example8/right.ipynb

The direct reason for exception is that id is not on the whitelist list:

nbdime/packages/nbdime/src/merge/model/cell.ts

Lines 154 to 160 in ed70f8d

	// TODO: Remove/extend whitelist once we support more
	super(base, [], mimetype, [
	'source',
	'metadata',
	'outputs',
	'execution_count',
	]);

The exact mechanism in which this gets triggered is not clear to me:

• example8 includes added/deleted cells and does not work
• example9 includes a single cell works fine despite changed IDs
• example10 includes a single cell and does not work

It might be that #639 solves this issue, but ideally all the examples would be covered by an integration test so that we know for sure.

[vidartf]

I think for 3.2, failing is probably the correct behavior, as we don't currently have a way of letting the user know that there is a conflict on a cell ID (nor does the code know what it means to have a conflict on a field that it doesn't know about). However, I think our failure UX needs a good revamp. I.e. we should better surface the error to the user without it hanging and having the user need to look in the browser webtools.

Separately, I think #639 does solve it since it marks the id field as immutable, meaning that the conflict will always be a decision on the cell path. Having a test for both of these scenarios makes sense to me:

• An integration test where the outputs of all the merge/diff test sets in nbdime/tests/files can be rendered in the diff/web view without error.
• A UX test for the web view on how it deals with a conflict on an unknown cell field "foo" + any other pertinent exceptions thrown during processing/rendering.

[vidartf]

This should now be fixed in master after merging #639. I will leave this issue open for now for visibility, and for considering backporting to lab-3 compatible version.

[vidartf]

As we are in rc now, I'm closing this. Backport can be considered later if needed.