Skip to content

fix(utils): correct misleading port-range comment in calculatePortOffset#1009

Merged
Wirasm merged 1 commit intocoleam00:devfrom
kuishou68:fix/issue-1008-port-range-comment
Apr 9, 2026
Merged

fix(utils): correct misleading port-range comment in calculatePortOffset#1009
Wirasm merged 1 commit intocoleam00:devfrom
kuishou68:fix/issue-1008-port-range-comment

Conversation

@kuishou68
Copy link
Copy Markdown
Contributor

@kuishou68 kuishou68 commented Apr 9, 2026

Closes #1008

Problem

The inline comment in calculatePortOffset says ports will be in 3100-3999 range, but with basePort = 3090 the actual range is 3190-4089. This contradicts the comment and confuses developers reading the code.

Fix

Updated the inline comment to correctly state 3190-4089 range. The JSDoc above the function was already accurate.

Summary by CodeRabbit

  • Chores
    • Improved internal documentation clarity for port allocation logic.

The inline comment stated ports 3100-3999 (implying basePort=3000) but the
actual basePort is 3090, making the true range 3190-4089, which already
matched the JSDoc. Update the inline comment to agree with both the JSDoc
and the arithmetic.

Closes coleam00#1008
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2c538e53-29a1-4739-af2b-a4d4ef539e31

📥 Commits

Reviewing files that changed from the base of the PR and between 50f96f8 and 9f6b438.

📒 Files selected for processing (1)
  • packages/core/src/utils/port-allocation.ts

📝 Walkthrough

Walkthrough

This change corrects a misleading inline comment in the calculatePortOffset function to accurately reflect the actual port range produced by the code (3190–4089 instead of 3100–3999), aligning it with the existing JSDoc documentation and the basePort value of 3090.

Changes

Cohort / File(s) Summary
Port Allocation Comment Fix
packages/core/src/utils/port-allocation.ts
Updated inline comment to accurately describe the resulting port range (3190-4089) when offset is added to basePort (3090), correcting the misleading range previously stated in the comment.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 Through the code warren I did hop,
Found a comment that made my ears flop!
3190 through 4089 rings true,
Not 3100's misleading view!
Truth and docs now both align,
My little nose twitches—all is fine! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: correcting a misleading comment about port range in the calculatePortOffset function.
Description check ✅ Passed The PR description clearly explains the problem, the fix, and references the linked issue, though it lacks the structured template sections like validation evidence and security impact.
Linked Issues check ✅ Passed The PR directly addresses issue #1008 by updating the inline comment to correctly state the 3190-4089 port range, aligning with the actual basePort value and existing JSDoc.
Out of Scope Changes check ✅ Passed The changes are strictly scoped to updating the misleading inline comment in calculatePortOffset with no behavioral code modifications or unrelated alterations.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@Wirasm Wirasm merged commit 0d1ff41 into coleam00:dev Apr 9, 2026
3 of 4 checks passed
leex279 pushed a commit that referenced this pull request Apr 9, 2026
…set (#1009)

The inline comment stated ports 3100-3999 (implying basePort=3000) but the
actual basePort is 3090, making the true range 3190-4089, which already
matched the JSDoc. Update the inline comment to agree with both the JSDoc
and the arithmetic.

Closes #1008
imagicrafter pushed a commit to imagicrafter/archon that referenced this pull request Apr 9, 2026
…feat-claude-sdk-node-options

feat(workflows): expose Claude SDK node options per DAG node
Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026
…feat-claude-sdk-node-options

feat(workflows): expose Claude SDK node options per DAG node
Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026
…set (coleam00#1009)

The inline comment stated ports 3100-3999 (implying basePort=3000) but the
actual basePort is 3090, making the true range 3190-4089, which already
matched the JSDoc. Update the inline comment to agree with both the JSDoc
and the arithmetic.

Closes coleam00#1008
joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026
…feat-claude-sdk-node-options

feat(workflows): expose Claude SDK node options per DAG node
joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026
…set (coleam00#1009)

The inline comment stated ports 3100-3999 (implying basePort=3000) but the
actual basePort is 3090, making the true range 3190-4089, which already
matched the JSDoc. Update the inline comment to agree with both the JSDoc
and the arithmetic.

Closes coleam00#1008
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.

fix(utils): misleading port-range comment in calculatePortOffset disagrees with actual basePort

2 participants