feat: add eval for Optional upsert side-effect boundary

[martinfrancois]

Problem

Add an eval for an Optional-based upsert flow where the code is functionally correct but harder to read because it checks one Optional-derived predicate and then reuses the same Optional in a second pass for side effects.

The skill should prefer one clear Optional decision boundary for present-vs-absent state, with named side-effect helpers for the present branch.

Java baseline: Java 25. Optional.ifPresentOrElse(...) is available.

Code before the prompt was executed

The code looked up an existing managed comment. If the existing comment already had the desired text, it returned. Otherwise, it filtered the same Optional for an updateable id and used ifPresentOrElse(...) to either update or create.

private void upsertPrerequisiteWaitingComment(EffectiveConfig config, Card card, String text) {
    try {
        Optional<Card.Comment> existing = prerequisiteWaitingComment(config, card.id());
        if (existing.map(Card.Comment::text).filter(text::equals).isPresent()) {
            return;
        }
        existing.filter(comment -> !blank(comment.id()))
                .ifPresentOrElse(
                        comment -> updateComment(config, comment.id(), text),
                        () -> addComment(config, card.id(), text));
    } catch (RuntimeException e) {
        LOG.warnf("card_id=%s prerequisite_waiting_comment=failed reason=%s", card.id(), e.getMessage());
    }
}

Prompt that caused the implementation

A maintainer asked:

the implementation of upsertPrerequisiteWaitingComment(EffectiveConfig looks a bit smelly, can you have a look over that with the java optionals skill and refactor it if its possible to improve?

Relevant behavior to preserve:

• If there is no existing managed comment, create one.
• If the existing managed comment already has the desired text, do nothing.
• If the existing managed comment has different text and a nonblank id, update it.
• If the existing managed comment has different text but cannot be updated because its id is blank, create a new comment.
• Keep runtime exception handling at the upsert boundary.

Prompt-produced code before maintainer correction

The reviewed code is the pre-prompt code above. It was correct, but it had an Optional readability smell.

Why the prompt-produced code is weak

The code is not weak because isPresent() is always forbidden. This isPresent() only answers a predicate and does not reopen the value with get().

The weakness is that the same Optional is consumed in two separate decisions:

1. First, existing.map(Card.Comment::text).filter(text::equals).isPresent() checks whether no work is needed.
2. Then, existing.filter(...).ifPresentOrElse(...) decides whether to update or create.

That makes the upsert policy harder to read because the present-vs-absent boundary is split across two Optional pipelines, and the id/text policy is embedded in the calling method instead of named.

Maintainer-preferred code

private void upsertPrerequisiteWaitingComment(EffectiveConfig config, Card card, String text) {
    try {
        prerequisiteWaitingComment(config, card.id())
                .ifPresentOrElse(
                        comment -> updateOrCreatePrerequisiteWaitingComment(config, card, comment, text),
                        () -> addComment(config, card.id(), text));
    } catch (RuntimeException e) {
        LOG.warnf("card_id=%s prerequisite_waiting_comment=failed reason=%s", card.id(), e.getMessage());
    }
}

private void updateOrCreatePrerequisiteWaitingComment(
        EffectiveConfig config, Card card, Card.Comment existing, String text) {
    if (text.equals(existing.text())) {
        return;
    }
    if (blank(existing.id())) {
        addComment(config, card.id(), text);
        return;
    }
    updateComment(config, existing.id(), text);
}

Why the replacement is better

The replacement keeps Optional as the single present-vs-absent decision boundary:

• present existing comment: handle update/create details in a domain-named helper;
• absent existing comment: lazily create the comment.

The helper makes the side-effect policy explicit: same text is a no-op, blank id falls back to create, and an updateable id updates.

This preserves behavior while making the Optional pipeline responsible only for optionality, not for every branch of the upsert policy.

Desired eval behavior

• Reward one clear Optional terminal for present-vs-absent side-effect flow.
• Reward moving nontrivial present-branch policy into a domain-named helper.
• Reward preserving side effects and branch order.
• Reward explaining that the original code is functionally correct but less maintainable.

Anti-patterns the eval should reject

• Reusing the same Optional in multiple pipelines when one decision boundary would be clearer.
• Replacing this with orElse(null) and local null control flow.
• Replacing this with isPresent() plus get().
• Creating generic Optional helper abstractions only to avoid ifPresentOrElse(...).
• Treating every isPresent() predicate as equally bad without explaining the actual maintainability issue.

Suggested eval name

optional-upsert-side-effect-boundary