Skip to content

feat: MetaflowTaggingError formatting and add test coverage for tagging_util#3165

Open
Vaishnav88sk wants to merge 2 commits into
Netflix:masterfrom
Vaishnav88sk:feat/add-tagging-util-tests
Open

feat: MetaflowTaggingError formatting and add test coverage for tagging_util#3165
Vaishnav88sk wants to merge 2 commits into
Netflix:masterfrom
Vaishnav88sk:feat/add-tagging-util-tests

Conversation

@Vaishnav88sk
Copy link
Copy Markdown

The validate_tag function was passing arguments incorrectly to MetaflowTaggingError, causing str() to fail when the exception was rendered. This also adds comprehensive test coverage for is_utf8_encodable, is_utf8_decodable, validate_tag, and validate_tags including edge cases for type validation, length limits, and tag set size constraints.

Fixes error message formatting for non-string tag types.

PR Type

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

Summary

Fix error message formatting bug in validate_tag and add comprehensive unit test coverage for tagging_util.py (30 tests).

Issue

Fixes #3162

Context / Motivation

The validate_tag function passed two positional arguments to MetaflowTaggingError, but the exception class expects msg, lineno, source_file as positional args. This caused TypeError: %d format: a real number is required, not str when rendering the exception for non-string tag types.

Additionally, tagging_util.py had zero test coverage despite being
critical infrastructure for run/step tagging validation.

Reproduction

Runtime:

Commands to run:

# paste exact commands

Where evidence shows up:

Before (error / log snippet)
paste here
After (evidence that fix works)
paste here

Root Cause

Changes Made

  • Fixed MetaflowTaggingError call to use string formatting instead of multiple positional arguments
  • Added 30 tests covering:
    • is_utf8_encodable / is_utf8_decodable functions
    • validate_tag with valid inputs and all error conditions
    • validate_tags including tag set size limits and remediation logic

Why This Fix Is Correct

  • All 30 new tests pass
  • Tests cover edge cases: empty tags, oversized tags, non-string types, UTF-8 encoding/decoding, tag deduplication, and size limits

Failure Modes Considered

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

AI Tool Usage

  • No AI tools were used in this contribution
  • AI tools were used (describe below)

For formatting

The validate_tag function was passing arguments incorrectly to
MetaflowTaggingError, causing str() to fail when the exception
was rendered. This also adds comprehensive test coverage for
is_utf8_encodable, is_utf8_decodable, validate_tag, and
validate_tags including edge cases for type validation, length
limits, and tag set size constraints.

Fixes error message formatting for non-string tag types.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 29, 2026

Greptile Summary

This PR fixes a genuine bug in validate_tag: passing str(type(tag)) as the second positional argument to MetaflowTaggingError silently set lineno to a string, causing TypeError: %d format: a real number is required, not str in MetaflowException.__str__ whenever the error was rendered. The fix pre-formats the message string before passing it, which is correct. The new test file is comprehensive and all test assertions are logically sound.

Confidence Score: 5/5

Safe to merge — minimal, targeted fix with solid test coverage and no regressions.

The only change to production code is a one-line fix that correctly pre-evaluates a format string. All 30 new tests are logically correct. No security, data, or runtime concerns.

No files require special attention.

Important Files Changed

Filename Overview
metaflow/tagging_util.py One-line fix: replaces two-argument MetaflowTaggingError call with a pre-formatted string, correctly preventing TypeError when str tried to use the tag type as a %d lineno.
test/unit/test_tagging_util.py New test file with 30 tests covering is_utf8_encodable, is_utf8_decodable, validate_tag, and validate_tags including edge cases; one test name is slightly misleading but logic is correct.

Reviews (4): Last reviewed commit: "Merge branch 'master' into feat/add-tagg..." | Re-trigger Greptile

@Vaishnav88sk Vaishnav88sk force-pushed the feat/add-tagging-util-tests branch from 00785c5 to 0599df1 Compare May 1, 2026 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix MetaflowTaggingError formatting and add test coverage for tagging_util

1 participant