Skip to content

Add eval for batch reference lookup before rendering #41

Description

@martinfrancois

Problem

Add an eval for refactoring code that mixes pure extraction, stream pipelines, hidden network
lookups, and explicit writes.

The lesson is not "turn everything into streams". The better answer is to use streams only for the
global pure extraction step, keep Trello writes in explicit loops, and avoid helper methods that
look pure but perform network I/O per item.

This is eval-worthy because a plausible implementation can look clean locally while still doing
repeated hidden I/O. The skill should prefer a workflow where code analyzes all items, collects all
lookup IDs once, fetches once, and then enriches each item from the fetched data.

Code before the prompt was executed

The old code first batched prerequisite-card lookups, but prompt-reference rendering could still
fetch more cards per card. The promptReferences(...) helper looked like a rendering helper, but it
also performed additional fetchCardStatesByIds(...) calls.

private List<Card> enrichPrerequisites(
        EffectiveConfig config, List<Card> cards, boolean includeReferenceContext, Set<String> cardsWithComments) {
    if (cards.isEmpty()) {
        return List.of();
    }
    Map<String, PrerequisitePlan> plans = new LinkedHashMap<>();
    Map<String, TrelloCardReference> referencedCards = new LinkedHashMap<>();
    for (Card card : cards) {
        PrerequisitePlan plan = prerequisitePlan(card);
        plans.put(card.id(), plan);
        plan.items().stream()
                .map(TrelloChecklistClassifier.PrerequisiteItem::reference)
                .forEach(reference -> referencedCards.putIfAbsent(reference.key(), reference));
    }
    Map<String, CardLookupResult> lookupResults = referencedCards.isEmpty()
            ? Map.of()
            : fetchCardStatesByIds(
                    config,
                    referencedCards.values().stream()
                            .map(TrelloCardReference::lookupId)
                            .toList());
    List<Card> enriched = new ArrayList<>();
    for (Card card : cards) {
        PrerequisitePlan plan = plans.getOrDefault(card.id(), PrerequisitePlan.empty());
        List<Card.PrerequisiteProblem> problems = new ArrayList<>(plan.problems());
        List<BlockerRef> blockers = blockerRefs(config, card, plan, lookupResults, problems);
        syncPrerequisiteItems(config, card, plan, lookupResults, problems);
        List<Card.TrelloReference> references =
                includeReferenceContext ? promptReferences(config, card, plan, lookupResults) : List.of();
        Card next = card.withRelationships(card.checklists(), references, problems, blockers);
        syncPrerequisiteWaitingFeedback(config, next, cardsWithComments.contains(next.id()));
        enriched.add(next);
    }
    return enriched;
}
private List<Card.TrelloReference> promptReferences(
        EffectiveConfig config,
        Card card,
        PrerequisitePlan plan,
        Map<String, CardLookupResult> prerequisiteLookup) {
    Map<String, ReferencedText> references = new LinkedHashMap<>();
    addReferences(references, "title", card.title());
    addReferences(references, "description", card.description());
    for (Card.Checklist checklist : card.checklists()) {
        for (Card.ChecklistItem item : checklist.items()) {
            String source = "checklist:" + nullToEmpty(checklist.name());
            TrelloCardReferenceParser.exactReference(item.text())
                    .ifPresent(reference -> references.putIfAbsent(
                            reference.key(), new ReferencedText(source, item.text(), reference)));
            addReferences(references, source, item.text());
        }
    }
    for (Card.Comment comment : card.comments()) {
        addReferences(references, "comment", comment.text());
    }
    for (Card.Attachment attachment : card.attachments()) {
        addReferences(references, "attachment", attachment.name());
        addReferences(references, "attachment", attachment.url());
    }
    plan.items().stream()
            .map(TrelloChecklistClassifier.PrerequisiteItem::reference)
            .forEach(reference -> references.putIfAbsent(
                    reference.key(), new ReferencedText("checklist", reference.url(), reference)));
    if (references.isEmpty()) {
        return List.of();
    }
    Map<String, CardLookupResult> lookupResults = new LinkedHashMap<>(prerequisiteLookup);
    List<String> missingLookups = references.values().stream()
            .map(ReferencedText::reference)
            .map(TrelloCardReference::lookupId)
            .filter(lookup -> !lookupResults.containsKey(lookup))
            .distinct()
            .toList();
    if (!missingLookups.isEmpty()) {
        lookupResults.putAll(fetchCardStatesByIds(config, missingLookups));
    }
    return references.values().stream()
            .map(reference -> promptReference(
                    config,
                    reference,
                    lookupResults.get(reference.reference().lookupId())))
            .toList();
}

Prompt that caused the implementation

The review prompt said the methods were mixing pure analysis, reference extraction, Trello lookups,
Trello writes, and relationship rendering. It asked for this shape:

  1. Analyze all cards once.
  2. Collect all Trello lookup IDs once, including prompt references when requested.
  3. Fetch lookup results once.
  4. Enrich each card from the already available lookup data.
  5. Keep Trello writes explicit and out of stream pipelines.

It also warned that promptReferences(...) could trigger additional card-state lookups per card
while the top-level method had already batched prerequisite lookups.

Prompt-produced code before maintainer correction

There was no separate prompt-produced intermediate implementation in this case. The existing code
above was the code being corrected.

Why the old code is bad

The old code is not bad because it is functionally wrong for small inputs. It is bad because the I/O
boundary is misleading.

  1. promptReferences(...) looks like a pure rendering helper, but it can perform network lookups.
  2. Those lookups happen once per card, so the same prompt reference can be fetched repeatedly across
    one batch.
  3. The top-level method already did a prerequisite lookup batch, so the extra I/O is easy to miss.
  4. The stream inside promptReferences(...) only deduplicates missing lookups for one card. It does
    not deduplicate the whole batch.
  5. The method mixes extraction, lookup, rendering, and mutation-related prerequisite state.

Maintainer-preferred code

The preferred shape analyzes every card first, gathers all lookup IDs through one pure stream, fetches
once, and then enriches each card in a clear loop.

private List<Card> enrichPrerequisites(
        EffectiveConfig config, List<Card> cards, boolean includeReferenceContext, Set<String> cardsWithComments) {
    if (cards.isEmpty()) {
        return List.of();
    }
    List<PrerequisiteAnalysis> analyses = cards.stream()
            .map(card -> analyzePrerequisites(card, includeReferenceContext))
            .toList();
    Map<String, CardLookupResult> lookupResults = lookupReferencedCards(config, analyses);
    List<Card> enriched = new ArrayList<>();
    for (PrerequisiteAnalysis analysis : analyses) {
        enriched.add(enrichPrerequisiteCard(
                config,
                analysis,
                lookupResults,
                cardsWithComments.contains(analysis.card().id())));
    }
    return enriched;
}

private Map<String, CardLookupResult> lookupReferencedCards(
        EffectiveConfig config, List<PrerequisiteAnalysis> analyses) {
    List<String> lookupIds =
            analyses.stream().flatMap(TrelloClient::lookupIds).distinct().toList();
    return lookupIds.isEmpty() ? Map.of() : fetchCardStatesByIds(config, lookupIds);
}

private static Stream<String> lookupIds(PrerequisiteAnalysis analysis) {
    Stream<String> prerequisiteLookupIds = analysis.plan().items().stream()
            .map(TrelloChecklistClassifier.PrerequisiteItem::reference)
            .map(TrelloCardReference::lookupId);
    Stream<String> promptLookupIds = analysis.promptReferences().values().stream()
            .map(ReferencedText::reference)
            .map(TrelloCardReference::lookupId);
    return Stream.concat(prerequisiteLookupIds, promptLookupIds);
}

Reference extraction is kept pure and local. It uses a tiny accumulator instead of forcing all nested
card fields into a stream pipeline.

private static Map<String, ReferencedText> promptReferenceTexts(Card card, PrerequisitePlan plan) {
    ReferenceAccumulator references = new ReferenceAccumulator();
    references.add("title", card.title());
    references.add("description", card.description());
    for (Card.Checklist checklist : card.checklists()) {
        String source = "checklist:" + nullToEmpty(checklist.name());
        for (Card.ChecklistItem item : checklist.items()) {
            references.addExact(source, item.text());
            references.add(source, item.text());
        }
    }
    for (Card.Comment comment : card.comments()) {
        references.add("comment", comment.text());
    }
    for (Card.Attachment attachment : card.attachments()) {
        references.add("attachment", attachment.name());
        references.add("attachment", attachment.url());
    }
    plan.items().stream()
            .map(TrelloChecklistClassifier.PrerequisiteItem::reference)
            .forEach(reference -> references.add("checklist", reference.url(), reference));
    return references.asMap();
}

The Trello write remains explicit and outside streams.

private ResolvedPrerequisites resolvePrerequisites(
        EffectiveConfig config, Card card, PrerequisitePlan plan, Map<String, CardLookupResult> lookupResults) {
    List<Card.PrerequisiteProblem> problems = new ArrayList<>(plan.problems());
    Map<String, BlockerRef> blockers = LinkedHashMap.newLinkedHashMap(
            plan.items().size() + (plan.problems().isEmpty() ? 0 : 1));
    boolean continueChecklistSync = true;
    for (TrelloChecklistClassifier.PrerequisiteItem item : plan.items()) {
        TrelloCardReference reference = item.reference();
        CardLookupResult result = lookupResults.get(reference.lookupId());
        BlockerRef blocker = blockerRef(config, card, reference, result, problems);
        blockers.putIfAbsent(blocker.id() == null ? reference.key() : blocker.id(), blocker);
        if (continueChecklistSync) {
            continueChecklistSync = syncPrerequisiteItem(config, card, item, result, problems);
        }
    }
    return new ResolvedPrerequisites(List.copyOf(problems), List.copyOf(blockers.values()));
}

Why the replacement is better

The replacement uses streams where they fit: global, pure extraction of lookup IDs. The stream is now
doing useful batch-level work, because distinct() deduplicates across all cards before the network
lookup happens.

The replacement avoids streams where they make the code worse: nested card-field extraction with
special cases, and Trello checklist writes. Those stay imperative, explicit, and easy to audit.

The I/O boundary is now visible:

  • analyzePrerequisites(...) and promptReferenceTexts(...) are pure.
  • lookupReferencedCards(...) is the single read boundary for referenced cards.
  • resolvePrerequisites(...) is the explicit per-card blocker and checklist-sync boundary.
  • promptReferences(...) only renders from already available lookup results.

Desired eval behavior

  • Reward collecting all lookup IDs once before calling the lookup method.
  • Reward using a stream for pure batch-level lookup ID extraction.
  • Reward keeping writes out of stream pipelines.
  • Reward naming the phases so pure extraction, lookup, rendering, and mutation are separate.
  • Reward a regression test proving repeated prompt references across multiple cards are fetched once.

Anti-patterns the eval should reject

  • A helper named like a formatter or renderer that performs hidden network I/O.
  • Per-item or per-card lookup calls when the required IDs can be collected first.
  • A stream-heavy rewrite that hides Trello writes inside map, peek, or collectors.
  • A stream that deduplicates only within one item when the business boundary is the full batch.
  • A refactor that changes behavior by dropping prerequisite blockers, prompt references, or checklist sync failures.

Suggested eval name

batch-reference-lookup-before-rendering

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions