fix(client): guard against MetaflowObject subclasses with unrecognised _NAME

[dheerenmohta]

Problem

MetaflowObject._NAME defaults to "base". The pathspec component-count validation is a chain of if self._NAME == "flow" ... elif self._NAME == "run" ... branches. Any subclass that forgets to override _NAME (or sets it to an unrecognised value) silently skips all count validation and accepts any-length pathspec.

This is a silent failure: no exception is raised, the wrong object is looked up, and the error surfaces far later with a confusing traceback.

Fix

1. Add a _KNOWN_NAMES class attribute (frozenset of the five valid names).
2. Add an explicit guard at the start of the pathspec block:

if self._NAME not in self._KNOWN_NAMES:
    raise MetaflowInternalError(
        "MetaflowObject subclass '%s' has _NAME='%s' which is not a recognised ..."
    )

This converts a silent, hard-to-debug failure into an immediate, actionable error.

Fixes #948 (partial — defensive correctness)

[greptile-apps]

Greptile Summary

This PR adds a _KNOWN_NAMES frozenset to MetaflowObject and inserts an explicit guard that raises MetaflowInternalError when a subclass has an unrecognised _NAME and is instantiated with a pathspec — converting a previously silent validation skip into an immediate, actionable error.

• Adds _KNOWN_NAMES = frozenset({"flow", "run", "step", "task", "artifact"}) as a class-level attribute, well-placed beside _NAME.
• Guard fires in the if pathspec and _object is None branch; the else branch already had a fallthrough MetaflowInternalError for unknown names, so both construction paths are now guarded.
• One ordering edge case: if a broken subclass also passes attempt, the pre-existing attempt check fires first with a less-informative MetaflowNotFound message before the new guard is reached.

Confidence Score: 4/5

Safe to merge — the change is narrowly scoped to defensive error-raising and does not alter any existing execution path for well-formed objects.

The guard is logically correct for the stated scenario, the frozenset is immutable and cannot be accidentally mutated, and the else branch already handled unknown names. The only gap is that the new error can be shadowed by the earlier attempt check when both a bad _NAME and a non-None attempt are present simultaneously, which is a minor diagnostic annoyance rather than a correctness problem. No tests were added to lock in the new behaviour.

Only metaflow/client/core.py changed; the guard placement relative to the attempt check (lines 307–311) is worth a second look.

Important Files Changed

Filename	Overview
metaflow/client/core.py	Adds _KNOWN_NAMES frozenset and a guard that raises MetaflowInternalError when a subclass carries an unrecognised _NAME into the pathspec-validation branch; logic is correct, but the guard fires only in the pathspec+no-_object path and doesn't surface before the earlier attempt-validation check.

Comments Outside Diff (1)

1. metaflow/client/core.py, line 307-311 (link)

   Misleading error when bad _NAME + attempt are combined

   The new _KNOWN_NAMES guard fires only in the if pathspec and _object is None branch. If a subclass has _NAME = "base" and the caller also passes attempt=0, the earlier check at line 308 raises MetaflowNotFound("Attempts can only be specified for Task or DataArtifact") — which is technically true but masks the real problem (unrecognised _NAME). Consider duplicating or hoisting the _KNOWN_NAMES check to before the attempt validation block so the root cause is always reported first.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

_{Reviews (1): Last reviewed commit: "fix(client): guard against MetaflowObjec..." | Re-trigger Greptile}

[greptile-apps]

No test coverage for the new guard

The fix converts a silent failure into an explicit MetaflowInternalError, which is easy to verify, but no unit test is included. A minimal test that instantiates a dummy subclass with an unrecognised _NAME and asserts the new error is raised would lock in this behaviour and prevent future regressions.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[codecov]

Codecov Report

❌ Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@dddc0d5). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
metaflow/client/core.py	33.33%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #3262   +/-   ##
=========================================
  Coverage          ?   29.11%           
=========================================
  Files             ?      381           
  Lines             ?    52516           
  Branches          ?     9267           
=========================================
  Hits              ?    15290           
  Misses            ?    36187           
  Partials          ?     1039           

☔ View full report in Codecov by Harness.
📢 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.