fix #1161

[smallfangqwq]

Summary by CodeRabbit

• Bug Fixes
  • Strengthened validation for training node identifiers to ensure they are properly formatted as numbers.

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[coderabbitai]

Walkthrough

The training handler now enforces stricter type validation for DAG node identifiers. The change adds isNumber to the lodash import list and replaces a truthiness check on the _id field with an explicit number type assertion in the _parseDagJson method. This prevents DAG nodes with falsy but non-numeric IDs from being accepted during parsing.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title "fix #1161" is vague and does not convey what change was made; it only references an issue number without describing the actual fix.	Replace with a descriptive title that explains the fix, such as "Validate DAG node IDs as numbers in training handler" or "Tighten DAG input validation for node IDs."

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

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.

packages/hydrooj/src/handler/training.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[lichmiko]

I have read the CLA Document and I hereby sign the CLA

[coderabbitai]

🧹 Nitpick comments (1)

packages/hydrooj/src/handler/training.ts (1)

2-2: Tighten _id validation: lodash isNumber accepts NaN (and Infinity)

The _id: 0 fix is correct, but _.isNumber(NaN) returns true, so the current assert(isNumber(node._id), ...) would allow _id: NaN, which then stays NaN after _id: +node._id.

Suggested change

-assert(isNumber(node._id), '_id should be a number');
+assert(isNumber(node._id) && !Number.isNaN(node._id), '_id should be a number');

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/hydrooj/src/handler/training.ts` at line 2, The current check
assert(isNumber(node._id), ...) allows NaN/Infinity; tighten validation by
asserting Number.isFinite(+node._id) (or Number.isFinite(node._id) if you only
accept numeric types) before converting — i.e. replace the
assert(isNumber(node._id), ...) check with an assertion that uses
Number.isFinite on the numeric value and then keep the existing conversion _id:
+node._id so NaN/Infinity are rejected.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/hydrooj/src/handler/training.ts`:
- Line 2: The current check assert(isNumber(node._id), ...) allows NaN/Infinity;
tighten validation by asserting Number.isFinite(+node._id) (or
Number.isFinite(node._id) if you only accept numeric types) before converting —
i.e. replace the assert(isNumber(node._id), ...) check with an assertion that
uses Number.isFinite on the numeric value and then keep the existing conversion
_id: +node._id so NaN/Infinity are rejected.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a297593f-18b8-41e5-b6c0-0cd2cb575e71

📥 Commits

Reviewing files that changed from the base of the PR and between 44b8c23 and 06f361e.

📒 Files selected for processing (1)

• packages/hydrooj/src/handler/training.ts