Skip to content

feat: wire-format cleanup — drop body projectId, lock timestamp format (#17, #20)#34

Merged
AndresL230 merged 5 commits into
mainfrom
feat/17-20-wire-format-cleanup
May 15, 2026
Merged

feat: wire-format cleanup — drop body projectId, lock timestamp format (#17, #20)#34
AndresL230 merged 5 commits into
mainfrom
feat/17-20-wire-format-cleanup

Conversation

@AndresL230
Copy link
Copy Markdown
Contributor

@AndresL230 AndresL230 commented May 15, 2026

Summary

  • Drops the redundant projectId field from the WindowSummary body. The URL path POST /projects/:id/telemetry remains the source of truth; the API has always ignored the body field. RecostConfig.projectId is unchanged.
  • Locks the timestamp wire format to ISO 8601 millisecond precision with UTC Z suffix (2026-05-14T12:00:00.000Z). Adds src/core/time.ts with an isoNow() helper documenting the contract; aggregator + interceptor route through it.
  • Tightens tests/contract.test.ts with two new regression guards: a strict ms+Z regex on both timestamps, and a JSON.stringify substring check that the body never carries projectId.

Closes #17. Closes #20.

Wave 2 of the issue-waves roadmap (docs/superpowers/roadmap-2026-05-13-issue-waves.md).

Cross-SDK follow-ups

Breaking changes (TypeScript-level)

  • WindowSummary.projectId is removed. TypeScript consumers reading summary.projectId will get a compile error pointing them here. Per WindowSummary body carries redundant projectId #17, the only known consumer is the ReCost API, which never read the body field.
  • AggregatorConfig.projectId is removed. The Aggregator constructor no longer accepts the option.

Test plan

  • npm run test — 237/237 green (228 baseline + 1 ms+Z regex + 1 no-projectId-substring + 7 dist)
  • npm run lint — clean
  • npm run build — clean dual ESM + CJS + DTS
  • Manual scratch-script verification: serialized payload contains no projectId; both timestamps match the locked regex ^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$

Summary by CodeRabbit

  • Documentation

    • Added implementation plan for wire-format cleanup initiative.
  • Bug Fixes

    • Removed projectId from event body; now retained only for internal routing.
  • Refactor

    • Standardized timestamp generation to use ISO 8601 UTC format with millisecond precision across the SDK.
    • Updated interface documentation to clarify timestamp format contract.
  • Tests

    • Updated test suite to reflect payload structure changes and validate timestamp format compliance.

Review Change Stack

AndresL230 and others added 5 commits May 14, 2026 20:03
8 tasks, 44 steps. Drops projectId from WindowSummary body and
locks the timestamp wire format to ms+Z via a dedicated isoNow()
helper + strict contract-test regex. Bundled single Node PR.
Cross-SDK Python mirror and API defensive-validator follow-ups
filed during Task 7.

Lands on the Wave 1 branch as a docs-only commit since PR #33 is
the active vehicle. Wave 2 implementation will start from a fresh
worktree off main once #33 merges.
…#20)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ormat (#20)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
URL path is the source of truth for projectId. The cloud API already
extracts it from `POST /projects/:id/telemetry` and ignores the body
field. Removing it from the wire payload eliminates the silent-mismatch
risk if a future API change started trusting the body field.

RecostConfig.projectId is unchanged (still required for cloud mode,
still drives the URL path via Transport).
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fdba03b5-6503-453c-a3bc-bd42fec43287

📥 Commits

Reviewing files that changed from the base of the PR and between c38595e and 401a1a8.

📒 Files selected for processing (10)
  • docs/superpowers/plans/2026-05-14-wire-format-cleanup.md
  • src/core/aggregator.ts
  • src/core/interceptor.ts
  • src/core/time.ts
  • src/core/types.ts
  • src/init.ts
  • tests/aggregator.test.ts
  • tests/contract.test.ts
  • tests/init.test.ts
  • tests/transport.test.ts
💤 Files with no reviewable changes (1)
  • src/init.ts

📝 Walkthrough

Walkthrough

This PR implements a wire-format cleanup that introduces centralized timestamp generation via isoNow() and removes the redundant projectId field from WindowSummary bodies. Timestamps are normalized to millisecond-precision ISO 8601 UTC format with Z suffix, while projectId is retained only in URL paths via the Transport layer. All production code and test suites are updated to reflect the new contract.

Changes

Wire-Format Standardization

Layer / File(s) Summary
Timestamp contract and helper
src/core/time.ts, src/core/types.ts
New isoNow() function exported from time.ts returns current UTC time as ISO 8601 string with millisecond precision and Z suffix. WindowSummary interface documentation tightened to specify millisecond-precision, Z-suffix contract for windowStart and windowEnd fields.
Timestamp emission refactoring
src/core/aggregator.ts, src/core/interceptor.ts
Aggregator.flush() and interceptor.buildEvent() refactored to use centralized isoNow() helper instead of inline new Date().toISOString() calls.
ProjectId removal from WindowSummary
src/core/aggregator.ts
AggregatorConfig interface removes optional projectId field. Aggregator removes internal _projectId field, constructor assignment, and projectId from flush() return object. ProjectId remains available via Transport layer's URL path construction from RecostConfig.projectId.
Aggregator and contract test updates
tests/aggregator.test.ts, tests/contract.test.ts
Aggregator tests updated to construct instances without projectId. Contract tests remove projectId from expected top-level schema, add strict YYYY-MM-DDTHH:mm:ss.SSSZ regex validation for both timestamp fields, and assert serialized body never contains projectId substring.
Init and transport test updates
tests/init.test.ts, tests/transport.test.ts
Init test refocused to validate environment forwarding to WindowSummary (removes projectId assertion). Transport tests uniformly use environment as payload discriminator instead of projectId across cloud POST, local-mode WebSocket, queue/drop, and overflow test scenarios.
Implementation plan documentation
docs/superpowers/plans/2026-05-14-wire-format-cleanup.md
Comprehensive specification enumerating eight implementation tasks: timestamp helper creation, refactoring emission sites, tightening contract assertions, removing projectId expectations, updating all test suites, README verification, cross-SDK follow-up issue templates, and final verification checklist.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

  • recost-dev/middleware-node#31 — Related to projectId handling in init.ts; this PR removes projectId from Aggregator configuration while the referenced PR adds early validation requiring projectId in cloud mode, suggesting complementary concerns around project ID lifecycle.

🐰 A timestamp walks into a wire,
"Now I'm ISO, you can't deny!"
ProjectId steps back with grace,
URL paths are its true place.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: removing projectId from WindowSummary body and standardizing timestamp format to ISO 8601 with Z suffix.
Linked Issues check ✅ Passed All code changes address #17 (projectId removal from WindowSummary) and #20 (timestamp format standardization to ms+Z). The implementation includes isoNow() helper, updated aggregator/interceptor/init, and comprehensive test updates validating the new contract.
Out of Scope Changes check ✅ Passed All changes are directly scoped to #17 and #20 objectives: removing projectId from body, introducing isoNow() timestamp helper, updating documentation and tests. No extraneous modifications detected.

✏️ 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 feat/17-20-wire-format-cleanup

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

❤️ Share

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

@AndresL230 AndresL230 merged commit 51040ec into main May 15, 2026
1 check passed
@AndresL230 AndresL230 deleted the feat/17-20-wire-format-cleanup branch May 21, 2026 04:15
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.

ISO 8601 timestamps emitted with different precision in Python vs Node WindowSummary body carries redundant projectId

1 participant