fix(client): avoid redundant metadata fetch in task log accessors

[ynachiket]

When loglines() or _log_size() already have meta_dict, resolve the log attempt from that dict instead of calling current_attempt, which re-fetches metadata_dict.

Fixes #3034

PR Type

• Bug fix
• New feature
• Core Runtime change (higher bar -- see CONTRIBUTING.md)
• Docs / tooling
• Refactoring

Summary

Accessing task logs via stdout/stderr (or log size) no longer triggers a redundant metadata_dict fetch when the log path already loaded metadata once.

Issue

Fixes #3034

Reproduction

Runtime: local (Client API + metadata provider; no flow execution required)

Commands to run:

cd test/unit
python -m pytest test/unit/test_task_log_metadata_fetch.py -v

Where evidence shows up: unit test assertions on Task.metadata_dict access count (proxy for metadata HTTP fetches in service-backed deployments)

Before (error / log snippet)

# Without the fix, loglines(meta_dict=...) calls self.current_attempt,
# which reads self.metadata_dict again when _attempt is None:

attempt = self.current_attempt  # -> metadata_dict property (2nd fetch)

FAILED: metadata_dict_mock.assert_not_called()  # AssertionError: called 1 time

After (evidence that fix works)

test/unit/test_task_log_metadata_fetch.py::test_loglines_does_not_refetch_metadata_when_meta_dict_provided[stdout] PASSED
test/unit/test_task_log_metadata_fetch.py::test_loglines_does_not_refetch_metadata_when_meta_dict_provided[stderr] PASSED
test/unit/test_task_log_metadata_fetch.py::test_log_size_does_not_refetch_metadata_when_meta_dict_provided[stdout] PASSED
test/unit/test_task_log_metadata_fetch.py::test_log_size_does_not_refetch_metadata_when_meta_dict_provided[stderr] PASSED
# metadata_dict_mock.assert_not_called() passes

Root Cause

_load_log() and _get_logsize() each call self.metadata_dict once, then pass that dict into loglines() / _log_size(). Both methods then resolved the log attempt via self.current_attempt. When Task was constructed without an explicit attempt, current_attempt reads self.metadata_dict again to get the "attempt" key—duplicating the metadata load (and, with a remote metadata service, an extra HTTP round-trip per log access).

Why This Fix Is Correct

_resolve_log_attempt(meta_dict) preserves the same attempt resolution order as current_attempt: explicit Task(..., attempt=N) wins; otherwise use meta_dict["attempt"] when meta_dict is already available; only fall back to current_attempt when no dict was supplied. The change is limited to the log accessor hot path and does not alter metadata semantics.

Failure Modes Considered

1. Explicit attempt on Task: Task(..., attempt=N) still takes precedence over meta_dict["attempt"] via _attempt is not None check.
2. Missing / legacy metadata: If meta_dict omits "attempt", behavior matches current_attempt (defaults to 0). If meta_dict is None, loglines() still loads metadata_dict once as before, then uses current_attempt only in that fallback path.

Tests

• Unit tests added/updated
• Reproduction script provided (required for Core Runtime)
• CI passes
• If tests are impractical: explain why below and provide manual evidence above

Non-Goals

• No changes to metadata service APIs, pagination, or latest_successful_run (Client API N+1 HTTP Query Storm: Unbounded Fetches + Client-Side-Only Filtering Cause O(n) Request Cascades #2942).
• No changes to legacy log_location_* code paths beyond shared attempt resolution.

AI Tool Usage

• No AI tools were used in this contribution

• AI tools were used (describe below)

• Tool(s): Cursor (AI assistant)

• Used for: identifying the redundant fetch from issue Redundant metadata_dict fetch in loglines() and _log_size() log loading path #3034, drafting _resolve_log_attempt, and initial unit test scaffolding

• Review: All code was reviewed, run locally (pytest test/unit/test_task_log_metadata_fetch.py -v, 7 passed), and adjusted before submit

[greptile-apps]

Greptile Summary

This PR eliminates a redundant metadata_dict fetch in Task.loglines() and Task._log_size() by introducing _resolve_log_attempt(meta_dict), which reads the attempt number directly from the already-loaded metadata dict instead of delegating to current_attempt (which re-fetches metadata_dict). The fix is narrowly scoped to the log-accessor hot path.

• Adds _resolve_log_attempt(meta_dict) that respects the same priority order as current_attempt (explicit _attempt > meta_dict["attempt"] > full fetch fallback).
• Replaces the self.current_attempt call in loglines and _log_size with self._resolve_log_attempt(meta_dict), saving one metadata round-trip per log access in service-backed deployments.
• Adds a new unit test file with 7 tests covering the no-refetch contract, the attempt-resolution priority, and the None-fallback path.

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped to the log-accessor hot path and preserves all existing attempt-resolution semantics.

The new _resolve_log_attempt helper faithfully replicates the priority order of current_attempt (explicit attempt > meta_dict["attempt"] > full-fetch fallback). Both call sites in loglines and _log_size already hold a freshly-fetched meta_dict, so the attempt value is identical to what the old code would have re-fetched.

No files require special attention.

Important Files Changed

Filename	Overview
metaflow/client/core.py	Adds _resolve_log_attempt helper and replaces two self.current_attempt calls in loglines/_log_size; logic is correct and preserves all existing resolution semantics.
test/unit/test_task_log_metadata_fetch.py	New regression test file covering the no-refetch contract for loglines and _log_size, attempt-resolution priority, and the None-fallback delegation path.

_{Reviews (3): Last reviewed commit: "test(client): align log metadata regress..." | Re-trigger Greptile}

[ynachiket]

Good call, let me know if there is any other feedback.

[talsperre]

If tests pass, I will merge it. Thanks for the PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (master@4871fb1). Learn more about missing BASE report.

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #3214   +/-   ##
=========================================
  Coverage          ?   28.36%           
=========================================
  Files             ?      381           
  Lines             ?    52359           
  Branches          ?     9244           
=========================================
  Hits              ?    14853           
  Misses            ?    36564           
  Partials          ?      942           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.