fix: Add worker and expectationManager configuration to troubleshoot#277
Open
fix: Add worker and expectationManager configuration to troubleshoot#277
Conversation
…data. This is a Quality of Life improvement
|
WalkthroughThis PR converts troubleshooting data collection to asynchronous operations across the package manager, enabling worker configuration fetching during troubleshooting. It adds a new Changes
Sequence DiagramsequenceDiagram
participant Client
participant CoreHandler
participant PackageManager
participant ExpectationManager
participant WorkerAgent
Client->>CoreHandler: troubleshoot()
activate CoreHandler
CoreHandler->>PackageManager: getDataSnapshot()
activate PackageManager
PackageManager->>ExpectationManager: getTroubleshootData()
activate ExpectationManager
ExpectationManager->>WorkerAgent: getConfiguration()
activate WorkerAgent
WorkerAgent-->>ExpectationManager: config (or error)
deactivate WorkerAgent
Note over ExpectationManager: Promise.all fetches<br/>all worker configs
ExpectationManager-->>PackageManager: { config, workers: [...] }
deactivate ExpectationManager
PackageManager-->>CoreHandler: snapshot with troubleshoot data
deactivate PackageManager
CoreHandler-->>Client: Promise<any>
deactivate CoreHandler
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shared/packages/worker/src/workerAgent.ts (1)
851-855: Consider documenting the type discriminator pattern.The wrapper uses property existence checks (
'cost' in result,'wipId' in result,'process' in result) to determine the return type and skip theknownReasoncheck. While this follows the existing pattern, it's somewhat fragile as it relies on specific properties being unique discriminators.The comment at line 854 helpfully identifies which type each property maps to. If
WorkerConfigis ever refactored to remove theprocessproperty, this check would silently break. Consider adding a type guard or ensuring these discriminator properties are documented as API contracts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shared/packages/worker/src/workerAgent.ts` around lines 851 - 855, The property-existence checks ('cost' in result, 'wipId' in result, 'process' in result) used in workerAgent.ts to discriminate return types are fragile; replace or reinforce them by adding explicit type guards and documenting the discriminator contract for types mapped to ExpectationCost, WorkInProgressInfo, and ReturnTypeGetConfiguration (e.g., create functions isExpectationCost(result): result is ExpectationCost, isWorkInProgressInfo(result): result is WorkInProgressInfo, isReturnTypeGetConfiguration(result): result is ReturnTypeGetConfiguration) and use those in place of the inline "'... in result'" checks, and also add a short comment or exported docstring on WorkerConfig listing the required discriminator properties so future refactors won’t silently break the knownReason logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@shared/packages/worker/src/workerAgent.ts`:
- Around line 851-855: The property-existence checks ('cost' in result, 'wipId'
in result, 'process' in result) used in workerAgent.ts to discriminate return
types are fragile; replace or reinforce them by adding explicit type guards and
documenting the discriminator contract for types mapped to ExpectationCost,
WorkInProgressInfo, and ReturnTypeGetConfiguration (e.g., create functions
isExpectationCost(result): result is ExpectationCost,
isWorkInProgressInfo(result): result is WorkInProgressInfo,
isReturnTypeGetConfiguration(result): result is ReturnTypeGetConfiguration) and
use those in place of the inline "'... in result'" checks, and also add a short
comment or exported docstring on WorkerConfig listing the required discriminator
properties so future refactors won’t silently break the knownReason logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 396b7f04-41cc-4278-80a2-26eaf223a2bd
📒 Files selected for processing (8)
apps/package-manager/packages/generic/src/coreHandler.tsapps/package-manager/packages/generic/src/generateExpectations/nrk/expectations.tsapps/package-manager/packages/generic/src/packageManager.tsshared/packages/api/src/methods.tsshared/packages/api/src/worker.tsshared/packages/expectationManager/src/expectationManager.tsshared/packages/expectationManager/src/workerAgentApi.tsshared/packages/worker/src/workerAgent.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
About Me
This pull request is posted on behalf of the NRK.
Type of Contribution
This is a: Code improvement
Current Behavior
The troubleshoot data returns various data points, but not the
configurationof ExpectationManager or Worker.New Behavior
Configuration of ExpectationManager or Worker is included in the troubleshoot data.
This is a Quality of Life improvement
Status