Skip to content

Address unresolved review concerns from PR #8447 and #8421#8451

Closed
Copilot wants to merge 2 commits intomasterfrom
copilot/review-comments-for-pull-8447
Closed

Address unresolved review concerns from PR #8447 and #8421#8451
Copilot wants to merge 2 commits intomasterfrom
copilot/review-comments-for-pull-8447

Conversation

Copy link

Copilot AI commented Mar 8, 2026

Several concerns raised in PR #8421 were never applied to the codebase, and PR #8447 introduced tests whose correctness was questioned. This applies all outstanding fixes.

query.js — ObjectID coercion guard

updateIdQuery was calling ObjectID() on any string _id, throwing on UUIDs and other non-hex formats.

// Before: blows up on UUIDs
if (query._id && query._id.length) {
  query._id = ObjectID(query._id);
}

// After: only coerces valid 24-char hex strings
const OBJECT_ID_PATTERN = /^[a-fA-F0-9]{24}$/;
if (query._id && typeof query._id === 'string' && OBJECT_ID_PATTERN.test(query._id)) {
  query._id = ObjectID(query._id);
}

Two tests now cover both cases — answering the reviewer's "Is this the correct requirement?" question: yes.

devicestatus.js — Refactor + truncatePredictions

  • Signature changed from storage(collection, ctx)storage(env, ctx); collection name extracted from env.devicestatus_collection internally
  • Added truncatePredictions(obj, maxSize) with full JSDoc as requested by the reviewer — documents the 288-point default (24 h × 5 min), supported prediction series (IOB, COB, UAM, ZT), and MongoDB 16 MB document-size motivation
  • Replaced the serial insertOne loop with a single insertMany call
  • remove now uses deleteMany + stat.deletedCount (legacy .remove() / stat.result.n dropped)

bootevent.js

Pass env instead of env.devicestatus_collection to devicestatus, matching the updated signature.

delaylist.jsauthFailDelay = 0 bypass

|| 5000 silently accepted 0 as a configured delay, eliminating auth-failure back-off. Replaced with an explicit positive-finite guard:

const rawAuthFailDelay = _.get(env, 'settings.authFailDelay');
const parsedAuthFailDelay = Number(rawAuthFailDelay);
const DELAY_ON_FAIL = Number.isFinite(parsedAuthFailDelay) && parsedAuthFailDelay > 0
  ? parsedAuthFailDelay
  : 5000;

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: bewest <394179+bewest@users.noreply.github.com>
Copilot AI changed the title [WIP] Review comments for pull request 8447 Address unresolved review concerns from PR #8447 and #8421 Mar 8, 2026
@AndyLow91 AndyLow91 closed this Mar 16, 2026
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.

3 participants