Skip to content

fix: correct TypeTag error message and harden vector parsing#248

Merged
beer-1 merged 2 commits intomainfrom
fix/typetag-bugs
Apr 7, 2026
Merged

fix: correct TypeTag error message and harden vector parsing#248
beer-1 merged 2 commits intomainfrom
fix/typetag-bugs

Conversation

@beer-1
Copy link
Copy Markdown
Member

@beer-1 beer-1 commented Mar 31, 2026

Summary

  • Fix typo: "known TypeTag""unknown TypeTag" in StringifyTypeTag error fallback
  • Fix TypeTagFromString vector parsing to correctly handle nested types and reject non-vector prefixes

Bug details

Bug 1: StringifyTypeTag returned errors.New("known TypeTag") for unrecognized variants — clearly should be "unknown".

Bug 2: TypeTagFromString used strings.HasPrefix(str, "vector") which would match any string starting with "vector" (e.g., a hypothetical struct vectorFoo). Changed to require both vector< prefix and > suffix, then extract inner type via str[7:len(str)-1].

This correctly handles nested vectors:

  • vector<u8> → inner: u8
  • vector<vector<u8>> → inner: vector<u8> → recursion works

Test plan

  • Added TestStringifyTypeTag_UnknownError — verifies corrected error message
  • Added TestTypeTagFromString_NestedVector — tests depth 1, 2, and 3 nested vectors

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Corrected the reported error message for unknown type-tag operations.
    • Strengthened validation for vector-type strings to require proper angle-bracket syntax.
  • Tests

    • Added tests covering type-tag error messages, nested vector parsing, and a negative regression for malformed vector strings.

… in TypeTagFromString

Fix "known TypeTag" -> "unknown TypeTag" error message and use precise
prefix/suffix matching for vector type parsing to avoid matching non-vector
types that happen to start with "vector".

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@beer-1 beer-1 requested a review from a team as a code owner March 31, 2026 05:23
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 31, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ec49c831-209f-421a-8b3f-29fb44e9f14c

📥 Commits

Reviewing files that changed from the base of the PR and between 1cae7f7 and bf15bcd.

📒 Files selected for processing (1)
  • api/exposed_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • api/exposed_test.go

Walkthrough

Updated vector type parsing to require exact vector<...> form and use direct slicing; corrected a fallback error message from "known TypeTag" to "unknown TypeTag". Added tests verifying the new error message and nested vector parsing (including a negative case).

Changes

Cohort / File(s) Summary
Error Message & Vector Parsing Improvements
api/exposed.go
Changed fallback error text in StringifyTypeTag to "unknown TypeTag". Tightened TypeTagFromString to require vector< prefix and > suffix and replaced trim-based substring extraction with direct slicing (str[7:len(str)-1]).
Test Coverage Expansion
api/exposed_test.go
Added TestStringifyTypeTag_UnknownError to assert the exact error message and TestTypeTagFromString_NestedVector to validate parsing of nested vector<...> forms and reject vectorFoo.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I munched a bug, found "known" was wrong,
Sliced vectors neat where they belong,
Tests hop in, three layers deep,
Now parsing wakes from sloppy sleep,
🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately summarizes the main changes: fixing the TypeTag error message typo and hardening vector parsing validation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/typetag-bugs

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
api/exposed_test.go (1)

92-119: Add a direct regression case for non-vector<...> prefixes.

Given the original bug, it’s worth explicitly asserting that inputs like vectorFoo are not treated as vectors.

✅ Suggested test extension
 func TestTypeTagFromString_NestedVector(t *testing.T) {
@@
 	inner3 := inner2.Value.(*types.TypeTag__Vector)
 	require.IsType(t, &types.TypeTag__U8{}, inner3.Value)
+
+	// regression: should not match arbitrary "vector" prefix
+	_, err = TypeTagFromString("vectorFoo")
+	require.Error(t, err)
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/exposed_test.go` around lines 92 - 119, Extend
TestTypeTagFromString_NestedVector to add a regression assertion that inputs not
using the proper "vector<...>" syntax (e.g. "vectorFoo") are not parsed as a
vector: call TypeTagFromString("vectorFoo") inside
TestTypeTagFromString_NestedVector and assert it returns an error
(require.Error) or at minimum that the returned tag is not a
*types.TypeTag__Vector (require.NotIsType), so the parser doesn't treat
"vectorFoo" as a vector type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@api/exposed_test.go`:
- Around line 92-119: Extend TestTypeTagFromString_NestedVector to add a
regression assertion that inputs not using the proper "vector<...>" syntax (e.g.
"vectorFoo") are not parsed as a vector: call TypeTagFromString("vectorFoo")
inside TestTypeTagFromString_NestedVector and assert it returns an error
(require.Error) or at minimum that the returned tag is not a
*types.TypeTag__Vector (require.NotIsType), so the parser doesn't treat
"vectorFoo" as a vector type.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 868f69b0-f3f5-4ced-b32d-fed68cc3b1e0

📥 Commits

Reviewing files that changed from the base of the PR and between c39ce30 and 1cae7f7.

📒 Files selected for processing (2)
  • api/exposed.go
  • api/exposed_test.go

Per CodeRabbit review, ensure that strings starting with "vector"
but not matching "vector<...>" syntax are not parsed as vector types.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@traviolus traviolus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@beer-1 beer-1 merged commit cf439b7 into main Apr 7, 2026
6 checks passed
@beer-1 beer-1 deleted the fix/typetag-bugs branch April 7, 2026 14:51
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.

2 participants