Conversation
Author
|
Closing as superseded by upstream commit 124a1ed, which normalizes null strategyContext and adds regression coverage. I checked current master (941c3be): generateContent now uses rawStrategyContext || {}, and the upstream test exercises strategyContext: null. Issue #38 is closed with the maintainer鈥檚 confirmation and CI link. This makes the fix in this PR redundant; no conflict-resolution commit is needed. Thank you for resolving it upstream. |
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
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.
What & why
Manual dashboard video creation sends
strategyContext: null. The existing destructuring default only handlesundefined, so generation crashes while readingstrategyContext.anglebefore reaching script writing. Normalize the optional context with?? {}so manual generation uses the existing strategy/channel defaults, while supplied strategy context remains intact.Closes #38.
Checklist
package-lock.jsonregeneration unless the PR is specifically about dependenciesnpm run lintpassesnpm testpasses (add a test if you fixed a bug)master(tested base260d7a94ab2d5bb2a98ce6620bfda7efd56ebd6b, still current immediately before publication)How I tested it
SystemTest.testManualGenerationStrategyContextto the standardnpm testrunner. It exercises the real strategy stage with null, omitted and populated contexts, checks defaults and supplied context preservation, and stops at the script-writing boundary before any provider/media work.TypeError: Cannot read properties of null (reading 'angle'). With the fix, it exits 0 for all three cases.npm test: 46 passed, 0 failed. The existing slideshow test verified browser-safe image embedding but skipped its live browser assertion because Chromium is not installed. No live provider/video generation was performed.npm run lintandgit diff --cached --check: passed.Scope: two files, 40 additions and one deletion; no dependency or generated-file changes.