New music analysis#8
Merged
Merged
Conversation
…ting. needs further testing and debugging
…changed folder structure), and system prompt changes for new music analysis
…pt + schema, and changed default model to 5.4-nano
…tral centroid, and form+ motion composition
… into new-music-analysis
…can actually debug
…param, dropped dead wave params, fixed move_z clipping to arena bounds instead of hardcoded value, matched collision check to MPC envelope, updated prompt for beat drop guidance
ratheron
requested changes
Jun 22, 2026
Comment on lines
+200
to
+201
| self._validate_known_uris("color_top", color_top.keys()) | ||
| self._validate_known_uris("color_bot", color_bot.keys()) |
Comment on lines
32
to
39
| @@ -33,7 +33,7 @@ def test_frontend_install_skips_cleanly_when_web_package_is_missing(tmp_path: Pa | |||
| bin_dir = tmp_path / "bin" | |||
| bin_dir.mkdir() | |||
| npm = bin_dir / "npm" | |||
| npm.write_text(f"#!/usr/bin/env bash\necho \"$@\" >> {npm_log}\n") | |||
| npm.write_text(f'#!/usr/bin/env bash\necho "$@" >> {npm_log}\n') | |||
Contributor
There was a problem hiding this comment.
The new version is not in accordance to our style guides. This is not detected because it's in test. Please revert anyway.
| _ALLIN1_IMPORT_ERROR = e | ||
| logger.error(f"{e} - please use the music env to analyze songs") | ||
| else: | ||
| _ALLIN1_IMPORT_ERROR = None |
Contributor
There was a problem hiding this comment.
We don't need to define this. In case we can't import here, log the error. When the program crashes further down, because allin1 is not define, it should be clear why it failed in the first place.
Comment on lines
+28
to
+35
| try: | ||
| import allin1 | ||
| except ImportError as e: | ||
| allin1 = None | ||
| _ALLIN1_IMPORT_ERROR = e | ||
| logger.error(f"{e} - please use the music env to analyze songs") | ||
| else: | ||
| _ALLIN1_IMPORT_ERROR = None |
Contributor
There was a problem hiding this comment.
Suggested change
| try: | |
| import allin1 | |
| except ImportError as e: | |
| allin1 = None | |
| _ALLIN1_IMPORT_ERROR = e | |
| logger.error(f"{e} - please use the music env to analyze songs") | |
| else: | |
| _ALLIN1_IMPORT_ERROR = None | |
| try: | |
| import allin1 | |
| except ImportError as e: | |
| logger.error(f"{e} - please use the music env to analyze songs") |
Comment on lines
+560
to
+563
| if allin1 is None: | ||
| raise ImportError("allin1 is required to analyze songs; please use the music env") from ( | ||
| _ALLIN1_IMPORT_ERROR | ||
| ) |
Contributor
There was a problem hiding this comment.
Suggested change
| if allin1 is None: | |
| raise ImportError("allin1 is required to analyze songs; please use the music env") from ( | |
| _ALLIN1_IMPORT_ERROR | |
| ) |
Comment on lines
+594
to
+597
| if allin1 is None: | ||
| raise ImportError("allin1 is required to analyze songs; please use the music env") from ( | ||
| _ALLIN1_IMPORT_ERROR | ||
| ) |
Contributor
There was a problem hiding this comment.
Suggested change
| if allin1 is None: | |
| raise ImportError("allin1 is required to analyze songs; please use the music env") from ( | |
| _ALLIN1_IMPORT_ERROR | |
| ) |
Contributor
There was a problem hiding this comment.
Since you changed the drones.toml here. Please also add the radio channel to the toml.
ratheron
approved these changes
Jun 22, 2026
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
2 participants
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.
No description provided.