Fix unused proto fields and hardcoded year in agent logic - #22
Open
KshitijKaushik123 wants to merge 1 commit into
Open
Fix unused proto fields and hardcoded year in agent logic#22KshitijKaushik123 wants to merge 1 commit into
KshitijKaushik123 wants to merge 1 commit into
Conversation
…nd MCP paths Several fields defined in the proto were not being used in the Go implementation, causing the agent to silently ignore request parameters: 1. applicable_intents was never read from the gRPC request — the agent returned all mutation types regardless of what the caller asked for. Now reads req.GetApplicableIntents() and passes it through to handlers for filtering. 2. MCP handler parsed lifecycle, originator, and applicable_intents from the tool arguments but never attached them to the gRPC request. Lifecycle was always UNSPECIFIED, originator was always nil, and intent filtering was completely bypassed through the MCP path. All three are now properly parsed and set on the request. 3. parseIntent was missing the ADD_CIDS case and parseLifecycle/parseOriginatorType functions didn't exist — added both. 4. Hardcoded year 2024 in demographic segment calculation meant age buckets would drift wrong every year. Now uses time.Now().Year(). Also includes proto regeneration (import path fix, go_package, Metric type path) and sample JSON bool fixes needed for the generated code to be in sync — these overlap with IABTechLab#11 but are included here since the code changes depend on the regenerated proto having the full field set.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Found several bugs where proto-defined fields were being parsed but never actually used, plus a hardcoded year that'll silently break demographic targeting every January.
Bug 1 —
applicable_intentsignored in gRPC pathThe agent was supposed to filter mutations based on
applicable_intentsfrom the request, but the code had avar applicableIntents []pb.Intent(always nil) with a TODO comment saying the field wasn't in the generated code yet. The field is in the proto and just needed a regen. With nil, every request got all mutation types back regardless of what was asked for — so a caller requesting onlyACTIVATE_SEGMENTSwould also getACTIVATE_DEALS,ADJUST_DEAL_FLOOR, etc.Bug 2 — MCP handler dropped lifecycle, originator, and applicable_intents
The MCP
extend_rtbhandler carefully parsed all three fields from the tool arguments but then built the gRPC request without any of them:lifecyclewas alwaysLIFECYCLE_UNSPECIFIEDregardless of inputoriginatorwas never set on the requestapplicable_intentswere parsed as strings but never converted to proto enums or attached to the requestThis meant the entire MCP path had no intent filtering and no lifecycle/originator context.
Bug 3 — Missing parsers and ADD_CIDS case
parseIntent()hadADD_CIDScommented out, andparseLifecycle()/parseOriginatorType()didn't exist at all — they were referenced in TODO comments but never written. Added all three.Bug 4 — Hardcoded year 2024 in age calculation
determineUserSegments()used2024 - user.GetYob()to calculate age for demographic bucketing. This means starting January 2025, a user born in 2000 would be calculated as 24 instead of 25, putting them in the wrong segment bucket. Changed totime.Now().Year().Proto regeneration
The generated Go code was out of sync with the proto definitions — fields like
applicable_intents,originator, and the fullLifecycleenum weren't in the generated structs. Regenerated with current protoc to bring them in sync. This overlaps somewhat with #11 but is included here because the code fixes depend on the regenerated fields being available.Testing
go build ./...passesgo vet ./...passesscripts/generate.shtwice produces same output)Note on #11
This PR includes the proto regeneration and sample JSON fixes that overlap with #11. If #11 lands first, the proto/sample changes here will merge cleanly since they produce the same result. The unique changes in this PR are the agent logic fixes (handlers.go, agent.go, mcp.go).