fix(rokt): stop emitting ATTRIBUTE_SETTER_CALLED diagnostic logs - #1412
Conversation
PR SummaryLow Risk Overview
Tests were updated to assert setters/identity no longer emit those codes, and removed coverage for the deleted builders. Reviewed by Cursor Bugbot for commit ed3f772. Bugbot is set up for automated code reviews on this repo. Configure here. |
📝 WalkthroughWalkthroughThe Rokt kit removes diagnostic timing for setter, identity, and placement-selection flows. Preselect diagnostics and placement diagnostic logging remain supported. Tests now validate the updated logging behavior. ChangesRokt diagnostic logging
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: 🔵 Low · up to The public comment should be narrowed to the payload requirement before merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
kits/rokt/src/diagnosticTiming.tsOops! Something went wrong! :( ESLint: 8.52.0 YAMLException: Cannot read config file: /kits/rokt/eslint.config.mjs 6 | import globals from 'globals'; Comment |
a113562 to
ed3f772
Compare
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
kits/rokt/src/diagnosticTiming.ts-3-3 (1)
3-3: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the logging implementation reference from this public comment.
The term
pipelinedescribes implementation structure. Keep the comment limited to the requirement that diagnostic payloads exclude attribute values.Proposed change
-// pipeline and payloads can carry customer PII. +// payloads can carry customer PII.Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: QUIET
Plan: Enterprise
Run ID: 4eabcffb-2111-4887-a9f4-18898e3d03f6
📒 Files selected for processing (3)
kits/rokt/src/Rokt-Kit.tskits/rokt/src/diagnosticTiming.tskits/rokt/test/src/diagnosticTiming.spec.ts
💤 Files with no reviewable changes (2)
- kits/rokt/test/src/diagnosticTiming.spec.ts
- kits/rokt/src/Rokt-Kit.ts
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.
## [3.2.1](v3.2.0...v3.2.1) (2026-09-11) ### Bug Fixes * **release:** wait for core npm visibility before publishing kits ([#1413](#1413)) ([1d5bf55](1d5bf55)) * **rokt:** stop emitting ATTRIBUTE_SETTER_CALLED diagnostic logs ([#1412](#1412)) ([d1e90dd](d1e90dd)), closes [#1405](#1405) [#1405](#1405) [#1398](#1398)
|
🎉 This PR is included in version 3.2.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Summary
v3-developmentafter #1398.mainand then reverted by #1408. A naive cherry-pick of fix(rokt): stop emitting ATTRIBUTE_SETTER_CALLED diagnostic logs #1405 would conflict with preselect (diagnosticTiming.tsmodify/delete).ATTRIBUTE_SETTER_CALLED/SELECT_PLACEMENTS_DISPATCHEDtiming diagnostics that were floodingecommerce-diagnostics-api.buildPreselectDiagnosticLogEntryandlogPlacementDiagnosticso preselectPRESELECT_*logs still work.Test plan
npx vitest runinkits/rokt— 380 tests passingselectPlacementspaths no longer emitATTRIBUTE_SETTER_CALLEDorSELECT_PLACEMENTS_DISPATCHEDPRESELECT_FIRED/MISSED/QUEUED/SKIPPED) still emitSummary by CodeRabbit