Skip to content

fix: pop opencode kwargs before passing to parent class#9

Merged
kalil0321 merged 1 commit into
kalil0321:mainfrom
puru2901is:main
Dec 27, 2025
Merged

fix: pop opencode kwargs before passing to parent class#9
kalil0321 merged 1 commit into
kalil0321:mainfrom
puru2901is:main

Conversation

@puru2901is
Copy link
Copy Markdown
Contributor

@puru2901is puru2901is commented Dec 27, 2025

OpenCodeEngineer.init() was passing opencode_provider and opencode_model to BaseEngineer.init(), which doesn't accept them. This caused a TypeError when using the opencode SDK. Fixed by using pop() to extract these kwargs before calling super().

Moved extraction of 'opencode_provider' and 'opencode_model' from kwargs to before the parent class initialization, using pop to avoid passing them to BaseEngineer. This ensures only relevant arguments are sent to the parent and clarifies initialization logic.

Greptile Summary

  • Refactored OpenCodeEngineer constructor to use kwargs.pop() for extracting OpenCode-specific parameters before parent class initialization
  • Changed parameter extraction for opencode_provider and opencode_model to prevent them from being passed to BaseEngineer

Important Files Changed

Filename Overview
src/reverse_api/opencode_engineer.py Refactored constructor to extract OpenCode-specific kwargs before parent initialization to prevent argument leakage

Confidence score: 5/5

  • This PR is safe to merge with minimal risk as it's a straightforward parameter handling improvement
  • Score reflects a defensive programming fix that prevents potential argument passing issues between parent and child classes
  • No files require special attention as the change is isolated and follows good OOP practices

Moved extraction of 'opencode_provider' and 'opencode_model' from kwargs to before the parent class initialization, using pop to avoid passing them to BaseEngineer. This ensures only relevant arguments are sent to the parent and clarifies initialization logic.
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (1)

  1. src/reverse_api/opencode_engineer.py, line 50 (link)

    style: After popping opencode_provider and opencode_model from kwargs, using kwargs.get('verbose', True) still works but could be inconsistent. Consider also popping 'verbose' if it's meant to be consumed here.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@puru2901is puru2901is changed the title Refactor OpenCodeEngineer init to pop specific kwargs fix: pop opencode kwargs before passing to parent class Dec 27, 2025
@kalil0321
Copy link
Copy Markdown
Owner

@puru2901is thank you! congrats on getting 5/ 5 (the first pr getting this score on the repo haha)!

@kalil0321 kalil0321 merged commit 4f4e98e into kalil0321:main Dec 27, 2025
2 checks passed
@puru2901is
Copy link
Copy Markdown
Contributor Author

puru2901is commented Dec 28, 2025

@puru2901is thank you! congrats on getting 5/ 5 (the first pr getting this score on the repo haha)!

@kalil0321 Thanks! Happy to contribute 🙌 Great project btw. I had fun debugging it while learning how it works!

kalil0321 added a commit that referenced this pull request May 5, 2026
…-version

Knocks out items #3, #4, #9 of the agent-friendliness backlog (#62).
Schema version stays at 1 since the contract has not shipped to prod —
fields are added in place rather than bumping versions.

Item #3 — Stable `usage` subset
  Different SDKs (Claude / OpenCode / Copilot) emit different keys for
  the same concepts (cache_creation_input_tokens vs cache_write_tokens,
  estimated_cost_usd vs total_cost vs total_cost_usd, etc). New helper
  `_normalize_usage()` maps SDK-native keys into a stable subset
  {input_tokens, output_tokens, cache_read_tokens, cache_write_tokens,
  total_cost_usd} and parks the SDK-native dict under `usage.raw` for
  power users. Wrappers can rely on the top-level keys without breaking
  when the user switches SDK.

Item #4 — Machine-readable `error_kind`
  Previously agents had to pattern-match on prose like "[Errno 13]
  Permission denied: '/x'" to decide whether to retry / abort / surface
  to the user. New `error_kind` field on every `agent --json` and
  `engineer --json` payload, with a fixed enum:
    misuse | config_invalid | permission_denied | network |
    engine_failure | interrupted | unknown
  Inferred via `_classify_error()` which uses isinstance checks on
  exceptions (KeyboardInterrupt, PermissionError, ConnectionError,
  TimeoutError) and substring fallback on plain messages. Misuse paths
  now pass `error_kind_hint="misuse"` explicitly.

Item #9 — `--json-schema-version`
  Top-level `reverse-api-engineer --json-schema-version` prints the
  schema version (currently `1`) and exits 0. Lets a wrapper query the
  contract version without having to invoke a real run.

Bonus: KeyboardInterrupt now formats as the literal "interrupted" in
the `error` field (str(KeyboardInterrupt()) is empty); empty exception
messages fall back to the class name.

Tests: 9 new tests in test_cli_followups.py (TestSchemaV2Normalization,
TestJsonSchemaVersionFlag) + updated existing tests to assert on the
normalized usage shape and the new error_kind field.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kalil0321 added a commit that referenced this pull request May 6, 2026
…-version

Knocks out items #3, #4, #9 of the agent-friendliness backlog (#62).
Schema version stays at 1 since the contract has not shipped to prod —
fields are added in place rather than bumping versions.

Item #3 — Stable `usage` subset
  Different SDKs (Claude / OpenCode / Copilot) emit different keys for
  the same concepts (cache_creation_input_tokens vs cache_write_tokens,
  estimated_cost_usd vs total_cost vs total_cost_usd, etc). New helper
  `_normalize_usage()` maps SDK-native keys into a stable subset
  {input_tokens, output_tokens, cache_read_tokens, cache_write_tokens,
  total_cost_usd} and parks the SDK-native dict under `usage.raw` for
  power users. Wrappers can rely on the top-level keys without breaking
  when the user switches SDK.

Item #4 — Machine-readable `error_kind`
  Previously agents had to pattern-match on prose like "[Errno 13]
  Permission denied: '/x'" to decide whether to retry / abort / surface
  to the user. New `error_kind` field on every `agent --json` and
  `engineer --json` payload, with a fixed enum:
    misuse | config_invalid | permission_denied | network |
    engine_failure | interrupted | unknown
  Inferred via `_classify_error()` which uses isinstance checks on
  exceptions (KeyboardInterrupt, PermissionError, ConnectionError,
  TimeoutError) and substring fallback on plain messages. Misuse paths
  now pass `error_kind_hint="misuse"` explicitly.

Item #9 — `--json-schema-version`
  Top-level `reverse-api-engineer --json-schema-version` prints the
  schema version (currently `1`) and exits 0. Lets a wrapper query the
  contract version without having to invoke a real run.

Bonus: KeyboardInterrupt now formats as the literal "interrupted" in
the `error` field (str(KeyboardInterrupt()) is empty); empty exception
messages fall back to the class name.

Tests: 9 new tests in test_cli_followups.py (TestSchemaV2Normalization,
TestJsonSchemaVersionFlag) + updated existing tests to assert on the
normalized usage shape and the new error_kind field.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

2 participants