Add generated MaxMind DB decoders - #221
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe change adds successor-validated cursor decoding, cursor-first custom unmarshaling, optimized decoder paths, and ChangesCursor decoding and decoder integration
Generator workflow
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Application
participant maxminddbGen
participant GeneratedDecoder
participant Cursor
Application->>maxminddbGen: Analyze source structs and generate methods
maxminddbGen->>GeneratedDecoder: Emit cursor and decoder methods
Application->>GeneratedDecoder: Decode an MMDB record
GeneratedDecoder->>Cursor: Traverse maps, slices, and scalar values
Cursor-->>GeneratedDecoder: Return validated successor cursors
GeneratedDecoder-->>Application: Populate the destination struct
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
5afd411 to
beb1a36
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/decoder/decoder.go`:
- Around line 418-432: Update UnexpectedKindError.Error() to format Actual and
Expected using Kind’s String()-based representation instead of numeric
formatting. Preserve the existing message structure so errors include readable
names such as “unexpected kind String, expected Map.”
In `@maxminddb-gen/generate.go`:
- Around line 706-723: Update the *types.Slice branch in the generator’s
byte-slice fast path to require an unnamed built-in byte element type, or
convert each []byte element to the target named element type before appending.
Preserve the existing optimized append behavior for exact []byte fields while
ensuring named byte slices such as []Octet generate compilable code.
In `@mmdbdata/error.go`:
- Around line 1-25: Add focused unit tests for the public helpers
NewUnmarshalTypeError and NormalizeUnmarshalError in a dedicated error test
file. Verify the generated error category and type details, normalization of an
UnexpectedKindError including wrapped errors, and unchanged propagation of
unrelated errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7ea6884e-b553-4de0-8ae2-09da9d87cb1d
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (34)
CHANGELOG.mdREADME.mdgo.modinternal/decoder/cursor.gointernal/decoder/cursor_external_test.gointernal/decoder/cursor_parity_test.gointernal/decoder/cursor_test.gointernal/decoder/data_decoder.gointernal/decoder/data_decoder_test.gointernal/decoder/decoder.gointernal/decoder/error_context.gointernal/decoder/nested_unmarshaler_test.gointernal/decoder/performance_test.gointernal/decoder/reflection.gointernal/decoder/testdata/fuzz/FuzzCursorBehavioralParity/19981bffc2abbaf1internal/decoder/testdata/fuzz/FuzzCursorBehavioralParity/56ce1d00fcf93ca2internal/decoder/testdata/fuzz/FuzzCursorBehavioralParity/5cfddfe87fbd8efbinternal/decoder/testdata/fuzz/FuzzCursorBehavioralParity/830e6b1f358378c7internal/decoder/testdata/fuzz/FuzzCursorBehavioralParity/a529f2bd56fdeb2einternal/decoder/testdata/fuzz/FuzzCursorBehavioralParity/a56fb6b698176e1dmaxminddb-gen/README.mdmaxminddb-gen/generate.gomaxminddb-gen/generate_test.gomaxminddb-gen/main.gomaxminddb-gen/rename_unix.gomaxminddb-gen/rename_windows.gomaxminddb-gen/testdata/basic/go.modmaxminddb-gen/testdata/basic/model.gomaxminddb-gen/testdata/basic/model_maxminddb.gomaxminddb-gen/testdata/basic/model_test.gommdbdata/doc.gommdbdata/error.gommdbdata/interface.gommdbdata/type.go
beb1a36 to
0c906da
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/decoder/performance_test.go`:
- Around line 150-178: Fix the unreachable slice fast path in Cursor.Slice by
reworking its kind detection to inspect the extended-kind byte before comparing
against KindSlice, or remove the dead branch if no valid fast path remains.
Ensure slice values no longer incorrectly fall through resolveCtrlData solely
because Kind(ctrlByte>>5) cannot represent KindSlice, and update
BenchmarkCursorOpenSmallContainers to exercise the intended path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b5524336-f092-4d46-856e-fbf4ca4776d9
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (28)
CHANGELOG.mdREADME.mdgo.modinternal/decoder/cursor.gointernal/decoder/cursor_external_test.gointernal/decoder/cursor_test.gointernal/decoder/data_decoder.gointernal/decoder/data_decoder_test.gointernal/decoder/decoder.gointernal/decoder/error_context.gointernal/decoder/nested_unmarshaler_test.gointernal/decoder/performance_test.gointernal/decoder/reflection.gomaxminddb-gen/README.mdmaxminddb-gen/generate.gomaxminddb-gen/generate_test.gomaxminddb-gen/main.gomaxminddb-gen/rename_unix.gomaxminddb-gen/rename_windows.gomaxminddb-gen/testdata/basic/go.modmaxminddb-gen/testdata/basic/model.gomaxminddb-gen/testdata/basic/model_maxminddb.gomaxminddb-gen/testdata/basic/model_test.gommdbdata/doc.gommdbdata/error.gommdbdata/error_test.gommdbdata/interface.gommdbdata/type.go
0c906da to
c05e169
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/decoder/data_decoder.go`:
- Around line 317-421: Replace the duplicated slow-path pointer resolution in
DataDecoder.decodeStringValue and decodeKey with DataDecoder.resolveCtrlData.
Use the helper’s resolved kind, size, data offset, and successor offset while
preserving existing pointer-to-pointer detection, error propagation, and type
validation behavior; leave the compact fast paths unchanged.
In `@README.md`:
- Around line 115-138: Update the go.mod example code fence in the README to use
go.mod or plain-text syntax instead of Go syntax, and remove the misleading //
go.mod marker from inside the block while preserving the tool directive content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2bd146cc-fa90-4339-8199-c7a413b88da9
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (28)
CHANGELOG.mdREADME.mdgo.modinternal/decoder/cursor.gointernal/decoder/cursor_external_test.gointernal/decoder/cursor_test.gointernal/decoder/data_decoder.gointernal/decoder/data_decoder_test.gointernal/decoder/decoder.gointernal/decoder/error_context.gointernal/decoder/nested_unmarshaler_test.gointernal/decoder/performance_test.gointernal/decoder/reflection.gomaxminddb-gen/README.mdmaxminddb-gen/generate.gomaxminddb-gen/generate_test.gomaxminddb-gen/main.gomaxminddb-gen/rename_unix.gomaxminddb-gen/rename_windows.gomaxminddb-gen/testdata/basic/go.modmaxminddb-gen/testdata/basic/model.gomaxminddb-gen/testdata/basic/model_maxminddb.gomaxminddb-gen/testdata/basic/model_test.gommdbdata/doc.gommdbdata/error.gommdbdata/error_test.gommdbdata/interface.gommdbdata/type.go
c05e169 to
17f99d7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/decoder/reflection.go`:
- Around line 93-99: Update the top-level CursorUnmarshaler branch in the
reflection decoding switch to route the returned successor through
Cursor.UnmarshalCursor, matching nested decoding validation. Ensure foreign,
zero, or otherwise invalid successors are rejected while preserving the existing
error propagation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6384b84a-40f3-47ab-b628-df0ce6cc24e6
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (28)
CHANGELOG.mdREADME.mdgo.modinternal/decoder/cursor.gointernal/decoder/cursor_external_test.gointernal/decoder/cursor_test.gointernal/decoder/data_decoder.gointernal/decoder/data_decoder_test.gointernal/decoder/decoder.gointernal/decoder/error_context.gointernal/decoder/nested_unmarshaler_test.gointernal/decoder/performance_test.gointernal/decoder/reflection.gomaxminddb-gen/README.mdmaxminddb-gen/generate.gomaxminddb-gen/generate_test.gomaxminddb-gen/main.gomaxminddb-gen/rename_unix.gomaxminddb-gen/rename_windows.gomaxminddb-gen/testdata/basic/go.modmaxminddb-gen/testdata/basic/model.gomaxminddb-gen/testdata/basic/model_maxminddb.gomaxminddb-gen/testdata/basic/model_test.gommdbdata/doc.gommdbdata/error.gommdbdata/error_test.gommdbdata/interface.gommdbdata/type.go
17f99d7 to
9af1db6
Compare
867c6b9 to
9db25f0
Compare
| @@ -15,6 +15,10 @@ func (d *Decoder) wrapError(err error) error { | |||
| // wrapErrorAtOffset wraps an error with context at a specific offset. | |||
| // Used when the error occurs at a different offset than the decoder's current position. | |||
| func (*Decoder) wrapErrorAtOffset(err error, offset uint) error { | |||
There was a problem hiding this comment.
It seems like we could eliminate this method entirely and just inline the new function.
|
|
||
| Import `github.com/oschwald/maxminddb-golang/v2/mmdbdata` for the decoder type. | ||
| For application-owned structs, `maxminddb-gen` can generate an | ||
| `UnmarshalMaxMindDB` method that avoids reflection. The generator is versioned |
There was a problem hiding this comment.
I think this is referring to the wrong method. Re-review everything in the README.md and CHANGELOG.md
0c1b73c to
aaf55f3
Compare
Add the cursor API and source-file generator needed to decode application models without reflection. Preserve reflection behavior, deterministic output, and safe regeneration.
Reject uninitialized public container cursors, extend reflection fallback and cross-platform generator coverage, and clarify cursor contracts.
aaf55f3 to
57b8ec9
Compare
Summary
Validation
Summary by CodeRabbit
maxminddb-genworkflow (withgo:generateand reproducible setup) to generate reflection-free unmarshaling for application-owned structs.UnexpectedKindError.maxminddb-gen/README.mdwith usage, output naming, and verification steps.