CSE Machine : Function Descriptions modification for Frame overlap

[Akshay-2007-1]

Description

This PR addresses the printable mode overlap issue identified in #3602 and is relevant to the issues #3624.

This PR:

• Moves function descriptions in Print mode to the bottom of function objects (instead of being to the right of function objects), which is also consistent with the textbook. (Refer to @martin-henz's comment ) This allows the CSE machine to be more compact.
• Implements a cap for function descriptions in Print mode, which was previously not implemented as seen below:

Before	After

• Assigns more vertical space for function objects to account for edge cases of overlapping function descriptions that involve arrays and nested recursive arrays/lists with closures. (Do note that though these scenarios seldom occur in the actual course work, we assume the worst case!)
• Do note that this elongation happens only in the Printable mode so that unnecessary visual gaps are not present in the normal mode where descriptions only appear on hovering!

Normal	Printable

Clearly, in the Printable mode we dynamically make sure that the height is MORE than the normal mode as illustrated above

• Before passing on the frames to the Layout team's implementation of the ghost algorithm, we change the widths of frames that includes the bindings and total widths of arrays including recursive calculations as well. This way, before the x-coordinates are fixed, we ensure that they are fixed in the way we want them to be making sure that no overlaps happen!

Before	After

• In the normal mode, for function descriptions that are very wide, even on hovering, we will only see the compact form of the description of that function object while for the full view, you can click on the object! This is demonstrated below :

Video.Project.3.mp4

• Earlier, the parantheses and the body closing curly bracket were not in place due to truncation but a recent commit fixed that!

Closes #3602
Closes #3624
Closes #3639

Concerns:

• Since more vertical space is allocated in frames to account for function descriptions being below function objects, there is now a new issue. After clicking Clear Dead Frames, if function objects are dead, it creates a very ugly elongated frame. This can be fixed in a future PR after all issues regarding the function description's layout is fixed and an issue will be made for the same soon.
  For the code below,

function foo(x) {
    return x;
}
const b = x => x => x => 5;
b(1)(1)(1);
function e() {
    return [() => 1, () => 1, () => 1];
}
e();
foo(1);
foo(() => 1);
foo(1);
const a = () => 5;
const c = () => 5;
const d = () => 5;

Before erasure	After erasure

• Sometimes, the width calculation done before passing on to the ghost algorithm for coordinate fixing goes a bit beyond what is expected and the visual gap increases more than expected in both the normal and the print modes! We are working towards fixing that. The same is illustrated below. (Credits to @ThatLi for the code being used as the test case here)

function foo() {
    function f(x) {
        return y => x + 3;
    }
    return f;
}
foo()(4);
foo()(5);

Before	After

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update
• Code quality improvements

How to test

Test our code out with any of the following code snippets. The expected result also is given right underneath!

Test 1

// Test B: sibling frames with closures + nested arrays
function make_frame(id, n) {
  const arr = [];
  let i = 0;
  while (i < n) {
    const k = id * 100 + i;
    function add(x) {
      return x + k;
    }
    function combo(x) {
      return add(x) + k;
    }
    arr[i] = [add, combo, [list(add, combo), [i, i + 1, i + 2]]];
    i = i + 1;
  }
  return () => arr;
}

const frames = [];
let j = 0;
while (j < 4) {
  frames[j] = make_frame(j, 4);
  j = j + 1;
}

frames[0]();
frames[1]();
frames[2]();
frames[3]();

Before	After

Do note that for the above case, I am attaching only a fraction of the Layout for demonstration sake, there is a lot more frames actually. Moreover, note that one must increase the maximum steps to 4000 before running the above code.

Test 2

const arr = [
  x => x + 1,
  [y => y * 2, [z => z - 3, 99]],
  5
];
arr;

Normal	Printable

Test 3

function pack(n) {
  return [x => x + n, [y => y * n, [z => z - n]]];
}
const a1 = pack(2);
const a2 = pack(3);
[a1, a2];

Normal	Printable

For more test cases, please refer to the ones given by @ThatLi at #3602. You will notice that they dont overlap anymore!

Checklist

• I have tested this code
• I have updated the documentation

[Akshay-2007-1]

The arrow team's fix that is awaited (detailed at #3633) can be made into a seperate PR.
Hence, we are getting this ready for review.
Some testing is awaited by @Oofky. Once that is down, it can be merged after review by maintainers!

[sentry]

Bug: In printable mode, the layout incorrectly reserves horizontal space for function descriptions, even though they are rendered vertically, causing excessive spacing between frames.
_{Severity: HIGH}

Suggested Fix

In Frame.tsx, when calculating valueRightX for FnValue or GlobalFnValue in printable mode, use value.x() + value.width() instead of value.x() + value.totalWidth. This will correctly use the function circle's width for horizontal spacing, ignoring the tooltip width which is not relevant as the tooltip is rendered below.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/features/cseMachine/components/values/FnValue.tsx#L103-L106

Potential issue: In printable mode, the `totalDataWidth` for `FnValue` and
`GlobalFnValue` is calculated using `value.totalWidth`, which incorrectly includes the
horizontal width of the function's description tooltip. However, in this mode, the
description is rendered vertically below the function, not horizontally beside it. This
miscalculation causes the layout engine to reserve unnecessary horizontal space between
frames, leading to an excessively wide diagram and undermining the goal of a more
compact printable view.

[coveralls]

Pull Request Test Coverage Report for Build 22877742440

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 72 of 112 (64.29%) changed or added relevant lines in 10 files are covered.
• 3 unchanged lines in 3 files lost coverage.
• Overall coverage increased (+0.05%) to 41.091%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/features/cseMachine/components/Frame.tsx	13	14	92.86%
src/pages/playground/Playground.tsx	0	1	0.0%
src/features/cseMachine/CseMachineUtils.ts	23	25	92.0%
src/features/cseMachine/CseMachine.tsx	14	20	70.0%
src/features/cseMachine/components/values/FnValue.tsx	6	21	28.57%
src/features/cseMachine/components/values/GlobalFnValue.tsx	6	21	28.57%

Files with Coverage Reduction	New Missed Lines	%
src/features/cseMachine/components/values/FnValue.tsx	1	63.25%
src/features/cseMachine/components/values/GlobalFnValue.tsx	1	52.44%
src/features/cseMachine/CseMachine.tsx	1	39.31%

Totals
Change from base Build 22873320594:	0.05%
Covered Lines:	5401
Relevant Lines:	12204

---

💛 - Coveralls

[martin-henz]

I just looked at #3646
I played around with examples and see that it does what it's supposed to do, so I approved it after glancing over the changes. It looks all reasonable to me.

I approve so we can merge. My hunch is that we should merge this so that the other teams can make use of the changes. I have a separate concern that I will discuss in telegram in in a future issue.

[RichDom2185]

@Akshay-2007-1 please make a PR to revert these changes.

[RichDom2185]

? Can we revert?

[RichDom2185]

???

[RichDom2185]

???!?!??!!