Skip to content

refactor(audit): address review comments on the audit consumer - #6958

Open
pfreixes wants to merge 1 commit into
masterfrom
pau/audit-consumer-followups
Open

refactor(audit): address review comments on the audit consumer#6958
pfreixes wants to merge 1 commit into
masterfrom
pau/audit-consumer-followups

Conversation

@pfreixes

@pfreixes pfreixes commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Follow-ups to review comments on #6954, which merged before they were addressed.

  • Drops a sort comparator and the comment defending it. Both guarded against a locale difference that cannot arise between pods running one image. Neither was needed either: the comparator only existed to satisfy a type-aware lint rule, and that rule had already stopped firing once the array was narrowed from (string | undefined)[] to string[] by a separate change.
  • Renames describe to nonIdentifyingFields. Its comment stated which fields may be read out of the audit blob — a constraint on the caller, not a description of what the function does. The name carries it instead. The token comment is trimmed to the part the code does not already say.
  • Removes the env test that only asserted zod rejects a fractional value. It exercised the library rather than any logic of ours.

No behaviour change: the same fields are logged, and the dedup token is byte-identical because Array.prototype.sort on a string array already compares UTF-16 code units.

Test plan

  • npm run ts-build clean
  • Unit: metering 6, utils 350, audit 25
  • Verified the token-stability test still fails if the sort is removed, so dropping the comparator did not weaken the guard
  • prettier + oxlint clean

Review in cubic

The sort comparator and its comment defended against a locale difference that cannot arise between
pods running one image. Neither was needed: the comparator only existed to satisfy a type-aware lint
rule that had already stopped firing once the array was narrowed to strings.

`describe` carried a comment about which fields may be read out of the blob, which is a constraint on
the caller rather than a description of the function. Renamed to `nonIdentifyingFields` so the name
carries it, and trimmed the token comment to the part the code does not already say.

Dropped the env test that only asserted zod rejects a fractional value.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 2 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@pfreixes
pfreixes requested a review from TBonnin July 30, 2026 13: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