Skip to content

core: typing for tsdoc#1164

Open
renbaoshuo wants to merge 2 commits into
hydro-dev:masterfrom
renbaoshuo:tsdoc-typing
Open

core: typing for tsdoc#1164
renbaoshuo wants to merge 2 commits into
hydro-dev:masterfrom
renbaoshuo:tsdoc-typing

Conversation

@renbaoshuo
Copy link
Copy Markdown
Contributor

@renbaoshuo renbaoshuo commented May 23, 2026

Summary by CodeRabbit

  • Refactor

    • Improved type safety and consistency for contest/training status and journal data
    • Standardized scoreboard cell values to strings for more consistent exports and display
    • Normalized status/index keys to stable string identifiers for template rendering
  • Bug Fixes

    • Ensured strict boolean handling when evaluating record view permissions

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 96053353-8acd-42be-ba4c-6f9363d792c5

📥 Commits

Reviewing files that changed from the base of the PR and between 1d6ce25 and 6a1d427.

📒 Files selected for processing (2)
  • packages/hydrooj/src/handler/homework.ts
  • packages/hydrooj/src/handler/training.ts

Walkthrough

Adds concrete contest/training status types (ContestJournalEntry, ContestDetailEntry, ContestStatusDoc, TrainingStatusDoc), updates ContestRule signatures to use ContestStatusDoc and journal arrays, propagates these types through handlers and models, stringifies scoreboard/export cells, keys several maps by ObjectId.toHexString(), and updates XCPCIO submission/journal handling and an upgrade script for type compatibility.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • hydro-dev/Hydro#1082: Refactors submissionBase and loadContestState in the same XCPCIO module to correctly derive per-entry submission and team context, sharing the same functional area for journal-based scoreboard construction.
  • hydro-dev/Hydro#1147: Overlaps on contest status typing and changes to contest lifecycle helpers such as isOngoing/isDone and status timestamp handling.
  • hydro-dev/Hydro#1036: Prior changes to contest handlers that interact with ContestDetailBaseHandler and contest status typing.

Suggested reviewers

  • undefined-moe
  • pandadtdyy
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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.
✅ 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 'core: typing for tsdoc' accurately reflects the main objective of this PR: introducing stronger TypeScript typing for tsdoc (status document) related types and handlers throughout the codebase.
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/homework.ts

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

packages/hydrooj/src/handler/training.ts

ESLint skipped: the ESLint configuration for this file 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

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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.

Inline comments:
In `@packages/hydrooj/src/model/contest.ts`:
- Around line 762-765: The code turns a valid 0 time into an empty string by
using || when building colTime; change the expression that defines colTime to
use nullish coalescing so zero is preserved (replace (tsddict[pid]?.time ||
'').toString() with (tsddict[pid]?.time ?? '').toString()). Update the related
variables (colTime, colTimeStr) usage so formatSeconds(colTime, false) still
receives the expected value; locate this in the block referencing tsddict, pid,
colTime, colTimeStr and formatSeconds in contest.ts and make the ?? change.

In `@packages/scoreboard-xcpcio/index.ts`:
- Line 50: The expression building teamId uses logical OR which treats 0 as
falsy and can result in undefined if ContestJournalEntry lacks uid; change the
fallback to use nullish coalescing so zero is preserved, e.g. compute teamId
from (uid ?? (rdoc as RecordDoc).uid) and then call toString() (or String(...))
to ensure a defined value; update the line that assigns teamId and ensure the
cast/rdoc access remains the same.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ef419a1b-cd2b-43db-bc48-a0da950579ae

📥 Commits

Reviewing files that changed from the base of the PR and between bbd7fbc and 1d6ce25.

📒 Files selected for processing (9)
  • packages/hydrooj/src/handler/contest.ts
  • packages/hydrooj/src/handler/problem.ts
  • packages/hydrooj/src/handler/record.ts
  • packages/hydrooj/src/interface.ts
  • packages/hydrooj/src/model/contest.ts
  • packages/hydrooj/src/model/document.ts
  • packages/hydrooj/src/model/training.ts
  • packages/hydrooj/src/upgrade.ts
  • packages/scoreboard-xcpcio/index.ts

Comment on lines +762 to 765
const colScore = (tsddict[pid]?.penaltyScore ?? '').toString();
const colOriginalScore = (tsddict[pid]?.score ?? '').toString();
const colTime = (tsddict[pid]?.time || '').toString();
const colTimeStr = colTime ? formatSeconds(colTime, false) : '';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Preserve zero-second submission times in homework rows.

At Line 764, || '' turns a valid 0 into an empty value, so exact-start submissions can lose time display/export data.

Suggested fix
-            const colScore = (tsddict[pid]?.penaltyScore ?? '').toString();
-            const colOriginalScore = (tsddict[pid]?.score ?? '').toString();
-            const colTime = (tsddict[pid]?.time || '').toString();
-            const colTimeStr = colTime ? formatSeconds(colTime, false) : '';
+            const colScore = (tsddict[pid]?.penaltyScore ?? '').toString();
+            const colOriginalScore = (tsddict[pid]?.score ?? '').toString();
+            const colTimeRaw = tsddict[pid]?.time;
+            const colTime = (colTimeRaw ?? '').toString();
+            const colTimeStr = colTimeRaw == null ? '' : formatSeconds(colTimeRaw, false);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const colScore = (tsddict[pid]?.penaltyScore ?? '').toString();
const colOriginalScore = (tsddict[pid]?.score ?? '').toString();
const colTime = (tsddict[pid]?.time || '').toString();
const colTimeStr = colTime ? formatSeconds(colTime, false) : '';
const colScore = (tsddict[pid]?.penaltyScore ?? '').toString();
const colOriginalScore = (tsddict[pid]?.score ?? '').toString();
const colTimeRaw = tsddict[pid]?.time;
const colTime = (colTimeRaw ?? '').toString();
const colTimeStr = colTimeRaw == null ? '' : formatSeconds(colTimeRaw, false);
🤖 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/model/contest.ts` around lines 762 - 765, The code turns
a valid 0 time into an empty string by using || when building colTime; change
the expression that defines colTime to use nullish coalescing so zero is
preserved (replace (tsddict[pid]?.time || '').toString() with
(tsddict[pid]?.time ?? '').toString()). Update the related variables (colTime,
colTimeStr) usage so formatSeconds(colTime, false) still receives the expected
value; locate this in the block referencing tsddict, pid, colTime, colTimeStr
and formatSeconds in contest.ts and make the ?? change.

const submit = new ObjectId(rdoc._id || (rdoc as any).rid).getTimestamp().getTime();
const id: ObjectId = '_id' in rdoc ? rdoc._id : rdoc.rid;
const submit = new ObjectId(id).getTimestamp().getTime();
const teamId = (uid || (rdoc as RecordDoc).uid).toString();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use nullish coalescing for team UID fallback to prevent runtime failure.

Line 50 can crash when uid is 0: uid || ... falls through, and ContestJournalEntry has no uid, so .toString() can run on undefined.

Proposed fix
-    const teamId = (uid || (rdoc as RecordDoc).uid).toString();
+    const teamUid = uid ?? ('uid' in rdoc ? rdoc.uid : undefined);
+    if (teamUid == null) throw new Error('Missing team uid for submission');
+    const teamId = teamUid.toString();
🤖 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/scoreboard-xcpcio/index.ts` at line 50, The expression building
teamId uses logical OR which treats 0 as falsy and can result in undefined if
ContestJournalEntry lacks uid; change the fallback to use nullish coalescing so
zero is preserved, e.g. compute teamId from (uid ?? (rdoc as RecordDoc).uid) and
then call toString() (or String(...)) to ensure a defined value; update the line
that assigns teamId and ensure the cast/rdoc access remains the same.

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.

1 participant