827 overuse of atsource boolean var and more

[aiAdrian]

This refactoring replaces the ambiguous atSource: boolean parameter with the explicit
TrainrunSectionNodeAnchor enum. This improves readability, reduces boolean ambiguity,
and makes the intent of each method call clearer. Several methods in
TrainrunSectionsView, PerlenketteSectionComponent, and related view logic were updated
accordingly to use the new enum-based API.

[louisgreiner]

After a very very short review, I think replacing the true/false values by an actual Type increases the readability, however the main issue remains. I'm not sure this is how we should treat that. Also, I don't really know what would be better 😄

@emersion @SharglutDev might have better insights, even tho the issue is more at a conceptual level

[SharglutDev]

I'm not familiar enough with the common guidelines in Angular or in the NGE repo.

If using enum is common, I agree that it improves readability but it might be cumbersome to use that everywhere ?

Another way we do in OSRD is using objects in function parameters:

getNode({ ..., atSource: boolean, ... }

This way when calling the function, we know to what the true/falserefer to (especially when multiple boolean param are used in function params) :

getNode({trainRunSection: this.trainrunSection, atSource: true, ...})

[aiAdrian]

I dont get the OSRD concept, how it fixes our issue. We might can dicuss this once we have more time, but i agree we need an improvment in readability.

[SharglutDev]

I dont get the OSRD concept, how it fixes our issue. We might can dicuss this once we have more time, but i agree we need an improvment in readability.

For example, some functions in NGE are like : function(true, false, false). This is not very readable, we need to go check what the function does and which param get which boolean.

With the object syntax, this would look like : function({a: true, b: false, c: false}). You directly see which is what (can also be solved with the enumproposal).

The initial "trigger" for me was the "double boolean", using the method isTip with isTip(true). This can be seen as "I want to specify that isTip is true" when in reality it means calling isTip with atSource: true.
So with the object syntax, when calling the method, it would be isTip({ atSource: true }).

[aiAdrian]

The introduction of an explicit AnchorType enum for source/target anchor nodes fits better than a { ... : boolean } argument approach from my point of view. Since the anchor node type is a core semantic element of the graph model, using an enum provides several advantages:

• Clearer intent: The meaning is encoded directly in the type instead of being inferred from booleans, which also resolves the readability issue.

• Explicit semantics: It becomes immediately clear which anchor node is used for the calculation.

For other method calls where the parameters are not tied to such fundamental domain concepts (e.g., not involving core identifiers like source/target), the lightweight { ... } object approach remains a good and pragmatic choice.

However, in this specific case, the enum makes the intended anchor node explicit and unambiguous, improving both clarity and maintainability.

Therefore, I vote for using this enum‑based approach, as it aligns the API with the semantic weight of the concept and supports long‑term robustness. (My personal opinion)

[SharglutDev]

Totally fine by me :)

[louisgreiner]

(I don't have to time to fully review)

The test files don't build and some typing issue lies in this PR (which are not detected since we do not have enough type checking in the CI).

I'm a bit worried about making these changes that affect so many things so quickly, I would recommend establish a clear strategy, and maybe split in smaller commit and make sure all the commits compile. This would ensure that we don't break too many things

[louisgreiner]

This does not return a boolean value, which is expected for setting the value of StaticDomTags.EDGE_IS_TARGET

[aiAdrian]

Very nice spot

[aiAdrian]

I fully agree that we shouldn’t merge this in a hurry — certainly not before we are ready for an internal release.
Realistically, the branch will only become mergeable once we have aligned on how to improve readability for these cases. In the meantime, any reviews are very welcome so we can settle the discussions before we start refactoring other similar methods.

[louisgreiner]

Except these two nits, this looks good to me. Maybe we can point all the places where we should use this, and list them in an refactor issue

[louisgreiner]

Nit: the name of this enum is a bit long. I also think "node" and "anchor" could refer to the same concept, we could choose only one. Maybe we could do something like:

Suggested change

	export enum TrainrunSectionNodeAnchor {
	export enum TrainrunSectionAnchor {

This is just a proposition, maybe we can find something better

[louisgreiner]

Nit: anchorNode is inconsistent regarding the name of the enum TrainrunSectionNodeAnchor

I would use either nodeAnchor (or anchor, if you accept my previous suggestion of enum renaming)