[Compyle] Task cmgnd0cd0000qq2iboczqm0ls updates for Ordo

[coderkrp]

Summary

• Files edited: 1
• Files added: 0
• Files deleted: 0
• Total diff: +108 / -41 lines

Base branch: master
Feature branch: compyle/cmgnd0cd0000qq2iboczqm0ls-2e8fda3

Edited files

• scripts/otp_cli.py

Summary by CodeRabbit

• New Features

  • Expanded login CLI to support broker-specific flows: OAuth-based, OTP-based, and mock.
  • Added new credential options (API key, username, password, API secret) with per-broker validation.
  • Added automatic parsing of redirect URLs to capture authorization details.
  • Improved messaging for unsupported brokers and clearer guidance during login.

• Refactor

  • Reworked login flow to branch by broker, aligning UX steps with each provider’s requirements.

[coderabbitai]

Walkthrough

Per-broker login flow added in scripts/otp_cli.py: new parse_redirect_url helper, expanded CLI options (broker-specific credentials), branching for fyers/hdfc/mock, and complete_login_api updated to accept optional auth parameters and build a conditional request body.

Changes

Cohort / File(s)	Summary
Auth completion request shaping scripts/otp_cli.py	complete_login_api(broker, session_data, auth_code: Optional[str]=None, response_state: Optional[str]=None) now constructs a conditional request_body and POSTs only provided fields instead of a fixed JSON payload.
Redirect URL parsing scripts/otp_cli.py	Added parse_redirect_url(redirect_url: str) -> Dict[str, str] to extract auth_code and state with parsing and error handling.
CLI options and validation scripts/otp_cli.py	login command extended with api_key, username, password, api_secret (HDFC-related options); added validation branches and explicit unsupported-broker handling.
Broker-specific login flow scripts/otp_cli.py	Login flow now branches by broker: fyers uses OAuth redirect URL prompt and parsing, hdfc uses OTP prompt/in-session OTP handling, mock bypasses extra input. Messaging and error paths updated accordingly.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor User
    participant CLI as otp_cli.py (login)
    participant API as Backend API

    User->>CLI: Run login --broker {fyers|hdfc|mock} [options]
    CLI->>API: initiate_login_api(broker, credentials)
    API-->>CLI: session_data

    alt fyers (OAuth)
        CLI->>User: Show auth URL, prompt for redirect URL
        User-->>CLI: Paste redirect URL
        CLI->>CLI: parse_redirect_url() => auth_code, state
        CLI->>API: complete_login_api(broker, session_data, auth_code, state)
        API-->>CLI: tokens / success
    else hdfc (OTP)
        CLI->>User: Prompt for OTP
        User-->>CLI: Enter OTP
        CLI->>API: complete_login_api(broker, session_data, auth_code=None) with OTP in request_body
        API-->>CLI: session confirmed
    else mock
        CLI->>API: complete_login_api(broker, session_data)
        API-->>CLI: mock success
    end

    CLI-->>User: Print result / errors

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

Thump-hop, I parsed a redirect line,
I nudged the OTP, I sipped the brine.
Fyers, HDFC, mock in tow—
I stitched the flows so they can go.
Ears up, nose twitch, the CLI's bright light. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check	❓ Inconclusive	The pull request title “[Compyle] Task cmgnd0cd0000qq2iboczqm0ls updates for Ordo” is too generic and auto-generated, failing to describe the primary changes such as the refactored otp_cli per-broker login flows and parsing helper. It does not highlight the main modifications to complete_login_api, parse_redirect_url function, or new broker-specific credential handling. Because it uses a task ID and non-descriptive phrasing, it is hard for teammates to quickly grasp the core purpose of this changeset.	Please update the title to a concise, descriptive summary of the main change (for example, “otp_cli: add per-broker login flows and parse_redirect_url helper”), omitting internal task IDs and using clear language.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch compyle/cmgnd0cd0000qq2iboczqm0ls-2e8fda3

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 907d7b3 and 3bb4004.

📒 Files selected for processing (1)

• scripts/otp_cli.py (3 hunks)

🔇 Additional comments (5)

scripts/otp_cli.py (5)

27-46: LGTM! Flexible API signature for multiple broker flows.

The conditional request body construction correctly handles different broker authentication patterns (OAuth for Fyers, OTP for HDFC, none for mock).

---

72-75: LGTM! HDFC CLI options properly defined.

The new broker-specific options are correctly typed and have clear help text.

---

149-152: Good fix! Unknown brokers now fall through to the API.

This correctly addresses the previous review feedback. The warning informs the user while allowing the server to be the authority on broker validation.

---

159-193: Broker-specific flows correctly implemented.

The three authentication patterns are clearly separated and handle each broker's requirements appropriately:

• Fyers: OAuth redirect flow
• HDFC: OTP-based flow
• Mock: Direct completion

The code is readable and maintainable despite some duplication in the complete_login_api calls.

---

138-143: Confirm HDFC credential key naming
The HDFC credentials dict mixes snake_case (api_key, username, password) with camelCase (apiSecret). Verify against the HDFC API spec whether the key should be api_secret and update accordingly.

---

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

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4209c22 and 907d7b3.

📒 Files selected for processing (1)

• scripts/otp_cli.py (3 hunks)

🧰 Additional context used

🪛 GitHub Actions: Python CI

scripts/otp_cli.py

[error] 151-151: Unsupported broker: invalid_broker. Supported brokers: fyers, hdfc, mock. This caused a typer.Exit(1) during test test_login_command_api_error, leading to test failure.