refactor(parser): extract extractEngineMCPSettings to bring extractEngineConfig under 60-line limit - #47009
Conversation
…gineConfig length Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Refactors parser engine configuration extraction to satisfy the 60-line linter limit without changing behavior.
Changes:
- Extracts MCP timeout handling into
extractEngineMCPSettings. - Preserves first-wins semantics for tool and session timeouts.
Show a summary per file
| File | Description |
|---|---|
pkg/parser/import_field_extractor.go |
Delegates engine MCP timeout extraction to a focused helper. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Medium
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. Test Quality Sentinel analysis not required. PR refactors pkg/parser/import_field_extractor.go (22 additions, 16 deletions) to extract extractEngineMCPSettings function—a production-only change. |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #47009 does not have the 'implementation' label and has only 22 new lines of code in business logic directories (threshold: 100). |
There was a problem hiding this comment.
Clean extract-method refactor. The new extractEngineMCPSettings helper is a faithful extraction of the original inline block — identical logic, same first-wins semantics, proper early return on type mismatch. No issues found.
🔎 Code quality review by PR Code Quality Reviewer · 9.61 AIC · ⌖ 4.33 AIC · ⊞ 5.6K
Comment /review to run again
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design — this is a clean, focused extraction with no logic change.
- ✅ Helper is correctly scoped as a method on
importAccumulator, consistent with adjacent extractors - ✅ Early-return guard for the type assertion replaces the nested
if ok— slightly cleaner - ✅
extractEngineConfignow sits comfortably under the 60-line limit
Nothing actionable to flag. LGTM.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 12.2 AIC · ⌖ 4.37 AIC · ⊞ 6.7K
Comment /matt to run again
There was a problem hiding this comment.
LGTM. Clean, behavior-preserving extraction of extractEngineMCPSettings. Logic, first-wins semantics, and logging are all faithfully preserved.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 10.7 AIC · ⌖ 4.32 AIC · ⊞ 5K
|
@copilot please run the Run: https://github.com/github/gh-aw/actions/runs/29813088952 Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "pi.dev"See Network Configuration for more information.
|
Done. I ran the PR-finisher flow and copilot-review checks.
Exact next maintainer action to merge: re-trigger CI on the current PR HEAD (close/reopen PR, |
|
@copilot please run the Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "pi.dev"See Network Configuration for more information.
|
Completed another
I cannot directly rerun required repository CI from this agent environment. Next maintainer action: re-trigger CI on the current PR HEAD (close/reopen PR, |
Branch is already current with I reran the PR-finisher checks for this HEAD:
This refactor is ready for maintainer review. If a fresh required-check run is still needed on this exact HEAD, the maintainer can re-trigger CI (close/reopen PR, |
|
🎉 This pull request is included in a new release. Release: |
extractEngineConfiginpkg/parser/import_field_extractor.gowas 67 lines, exceeding the 60-line custom linter threshold.Changes
pkg/parser/import_field_extractor.go— pulled the engine MCP timeout extraction block out ofextractEngineConfiginto a new private helperextractEngineMCPSettings(mcpVal any, fullPath string), reducingextractEngineConfigfrom 67 → 52 lines with no logic changeThe new helper encapsulates the first-wins extraction of
engine.mcp.tool-timeoutandengine.mcp.session-timeout, matching the pattern used by other focused extractors in this file.Run: https://github.com/github/gh-aw/actions/runs/29813088952
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
pi.devSee Network Configuration for more information.
Run: https://github.com/github/gh-aw/actions/runs/29816999996
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
pi.devSee Network Configuration for more information.