Skip to content

fix(typecheck): stabilize GRPO setup and scored-data typing#42

Open
Dexploarer wants to merge 5 commits intoJejuNetwork:developfrom
Dexploarer:codex/fix-grpo-typecheck
Open

fix(typecheck): stabilize GRPO setup and scored-data typing#42
Dexploarer wants to merge 5 commits intoJejuNetwork:developfrom
Dexploarer:codex/fix-grpo-typecheck

Conversation

@Dexploarer
Copy link
Contributor

Summary\n- Fix optional model attribute access in GRPO trainer setup by using local typed model refs before methods/properties are called.\n- Refactor Jeju RLAIF env scored-group typing to avoid invalid TypedDict inheritance and keep the inference logprobs payload typed via a local alias/cast.\n\n## Validation\n- bunx pyright packages/training/python apps/dws/api

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c36be009b7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 274 to 275
const nonce = bytesToHex(randomBytes(32))
const timestamp = Date.now()

Choose a reason for hiding this comment

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

P1 Badge Reuse external auth challenge across login invocations

The external flow generates a fresh nonce/timestamp (and therefore a different message) every time jeju login --external runs, but the UX is explicitly two-step (print message first, submit signature later). On the second invocation, verifyMessage is run against a new message, so signatures from the printed message cannot validate and external login cannot complete.

Useful? React with 👍 / 👎.

Comment on lines +291 to +292
`jeju login --network ${network} --address ${address} --signature <your-signature>`,
)

Choose a reason for hiding this comment

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

P2 Badge Include --external in the follow-up login command

The follow-up command shown to users omits --external, but signature handling only runs inside the if (options.external) branch. If users follow this instruction literally, the CLI skips external-signature verification and falls back to private-key auth, so the documented external flow fails.

Useful? React with 👍 / 👎.

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