You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
chore: close the restructuring's follow-ups (roles in the reader's language, e2e split, canvas ceiling, config readers) - #366
The loose ends the backend (#351–#358) and webapp (#359–#365) restructurings noted but did not fix, because those pull requests only moved code. One commit per subject.
Web client
Roles and the library count in the reader's language (fix(webapp)). The account rows printed the API's admin/user and a hard-coded English "N libraries"; the membership panel printed owner/manager/listener and offered them as the role picker's labels. They now go through the translations (admin.role*, admin.accountLibraries with its plural). English reads exactly as before; French reads administrateur · 1 bibliothèque, propriétaire, auditeur, gestionnaire. The <option> values are unchanged, so what is sent is unchanged.
Player (refactor(webapp)): the queueLength ref was written on every render and read nowhere — removed. The order effect's comment said the queue is compared "by identity and by prefix"; continuationOf compares entry by entry and its own comment says why a prefix test was dropped — the comment now says so. onEnd's "stay on the last track" explanation was written twice; the copy outside the handler is gone.
Queries: the filedTreeQuery doc comment sat above the "Needs attention" block, two doc comments stacked on one export; it is back on filedTreeQuery.
e2e
smart-playlists-and-podcasts.spec.ts (1,102 lines) split by subject into smart-playlists.spec.ts (234) and podcasts.spec.ts (452), with the shared mock in e2e/fixtures/smart-playlists-and-podcasts.ts (389). Its state was eight module-level lets, which an importing spec cannot reassign; they are now the fields of one state object, as fixtures/state.ts does for the rest, reset to the same eight values before each test. The layout gate's allowlist is now empty.
Copied helpers replaced by the fixtures, only where the copy was identical: session, albums, library(...), expectNoWcagViolations, navigateWithin, the one-pixel PNG (PIXEL, seven copies) and silentWav(30) (whose removed copy's comment said "one second"; it was thirty). Copies that differ (other defaults, other fields, inline accessibility checks with other options) are left as they are.
A test for the roles above, in French: names the roles and the library count in the reader's language.
Proof that nothing was lost: playwright test --list gave 508 tests before the split and 508 after, with the sorted titles identical; 510 with the new test (one test × two projects).
Server
The canvas body ceiling has a finite fallback (fix(app)). boot turned config.canvas.max_bytes (an i64) into the route's body ceiling with unwrap_or(usize::MAX). Config::from_env refuses a value that does not fit, but a Config built by hand never meets that check, and a negative value then meant no ceiling at all on the one route that takes a whole video in a body. It now goes through canvas_body_limit in app/state.rs, beside chunk_body_limit, and falls back to the default 4 MiB. Unit test a_canvas_size_that_does_not_fit_keeps_a_finite_ceiling; inverted: with the fallback put back to usize::MAX it fails.
config: seven settings where zero is a value (API_TOKEN_TTL_SECS, SCAN_INTERVAL_SECS, UNAVAILABLE_RETENTION_DAYS, PURGE_GRACE_HOURS, QUALITY_PARALLELISM, MIGRATION_BACKUPS, PODCAST_REFRESH_INTERVAL_SECS) each re-wrote var → default → parse → "invalid NAME: error" by hand, which is parse_env word for word. They call it now; defaults and error messages are unchanged.
cli: the module comment said the CLI was "used before the M4 web administration surface exists". It now says what it is for, and why restore and key run without the AppState.
CI
Both layout gates move to the repository root first. Run from a subfolder, their find met no file and they passed having measured nothing. Inverted for each: from webapp/, a 601-line src/lib file passes the old webapp gate and fails the new one; from src/, an 801-line file passes the old Rust gate and fails the new one.
webapp/: biome check (the same 2 warnings as main), bun run typecheck (src, node configs and e2e), bun run test327 passed, bun run build, scripts/check-webapp-layout.sh.
Playwright left to CI (local runs saturate the machine): the split specs and the new French admin test run here for the first time. The new test has not been inverted locally for the same reason.
Noticed, not changed
The smart-playlists/podcasts fixture and workspace-pages.spec.ts set the CSRF cookie for http://127.0.0.1:4173, ignoring baseURL, so under WAVEFLOW_E2E_PORT it would go to the wrong origin. Kept as it was: changing it changes behaviour.
workspace-pages.spec.ts has its own Episode type and channelOf, identical to the podcast fixture's; sharing them would make it import from the podcasts fixture.
The role picker's aria-label reads Rôle: guest in French, without the space French puts before a colon.
Summary by CodeRabbit
Nouvelles fonctionnalités
Les rôles et le nombre de bibliothèques des comptes sont affichés avec des libellés traduits en français et en anglais.
Les tests automatisés couvrent davantage les parcours d’administration, de playlists intelligentes et de podcasts.
Corrections
Les scripts de vérification fonctionnent désormais depuis n’importe quel répertoire.
Une limite de requête Canvas finie est conservée même si la valeur configurée ne peut pas être convertie.
La lecture et les podcasts bénéficient d’une couverture accrue des cas de fin de lecture et des requêtes tardives.
Documentation
La documentation de la CLI décrit davantage ses usages, notamment avant le démarrage du serveur.
…pec.ts
The file is about to be split by subject, and podcasts is the larger
half. Renamed alone, so that its history follows it; the layout
allowlist follows the path until the split removes the entry.
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
The spec was 1,102 lines, the one file the layout check allowed past
its limit. Its two subjects shared one mock whose state was
module-level `let`s, which an importing spec cannot reassign; that
state is now the fields of one object in
`fixtures/smart-playlists-and-podcasts.ts`, beside the mock itself and
a hook that resets every field and installs the routes, as the old
`beforeEach` did. `smart-playlists.spec.ts` and `podcasts.spec.ts`
each install it and hold their own tests, unchanged but for reading
and writing that object.
The spec's copy of `expectNoWcagViolations`, identical to the one in
`fixtures/accessibility.ts`, gives way to it. The allowlist entry goes; the mechanism
stays, with nothing on it.
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
Where a spec held its own copy of something `e2e/fixtures/` already
exports, and the copy is the same, the spec now imports it:
- `session` from `catalogue.ts`, in editorial-pages, themes,
workspace-pages and the smart playlists and podcasts mock;
- `albums` from `catalogue.ts`, in editorial-pages and themes, and the
themes spec's library from `library("library-1", "Ma musique")`,
which answers the same object;
- `expectNoWcagViolations` from `accessibility.ts`, in editorial-pages
and workspace-pages;
- `navigateWithin` from `radio.ts`, in podcasts;
- `PIXEL` from `media.ts`, for the one-pixel PNG built inline in
artist, editorial-pages, library-cache, player and podcasts;
- `silentWav(30)` from `media.ts`, for the thirty-second WAV themes and
the smart playlists and podcasts mock built by hand, byte for byte the
same.
Copies that differ from the fixture, by their fields or their
signature, stay where they are.
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.
Walkthrough
Le changement ajuste le calcul de la limite Canvas et le parsing de paramètres Rust. Il traduit les libellés des pages d’administration, étend les tests E2E des playlists et podcasts, et met à jour des scripts ainsi que des fixtures partagées.
Changes: Vérifications de structure du dépôt
Layer / File(s)
Summary
Exécution des contrôles depuis la racine scripts/check-rust-layout.sh, scripts/check-webapp-layout.sh
Les scripts se placent à la racine du dépôt avant leurs vérifications. Le script web retire l’exception du test des playlists intelligentes et des podcasts.
Configuration et initialisation Rust
Layer / File(s)
Summary
Lecture des paramètres d’environnement src/config/from_env.rs, src/config/podcasts.rs, src/cli/mod.rs
Le parsing de plusieurs paramètres utilise parse_env, avec les valeurs par défaut conservées. La documentation CLI décrit les commandes et les configurations utilisées.
Calcul de la limite du corps Canvas src/app/state.rs, src/app/boot.rs, src/app/tests.rs
canvas_body_limit convertit la limite configurée en usize et utilise 4 Mio si la conversion échoue. Le démarrage appelle cette fonction et un test vérifie les deux cas.
Traductions de l’administration
Layer / File(s)
Summary
Libellés et vérification en français webapp/src/state/i18n/{en,fr}/admin.ts, webapp/src/pages/settings/admin-page/{index.tsx,libraries.tsx}, webapp/e2e/admin.spec.ts
Les pages affichent des libellés traduits pour les rôles et le nombre de bibliothèques. Le test Playwright vérifie les libellés français et la valeur du sélecteur de rôle.
Couverture E2E des playlists et podcasts
Layer / File(s)
Summary
Réutilisation des fixtures E2E webapp/e2e/{artist,editorial-pages,library-cache,player,themes,workspace-pages}.spec.ts
Les tests utilisent des fixtures partagées de catalogue, de média et d’accessibilité à la place de données ou d’utilitaires locaux.
Mock stateful des API de playlists et podcasts webapp/e2e/fixtures/smart-playlists-and-podcasts.ts
La fixture ajoute des routes simulées qui conservent l’état des playlists, podcasts, épisodes et signets entre les requêtes.
Scénarios des playlists intelligentes webapp/e2e/smart-playlists.spec.ts, webapp/e2e/smart-playlists-and-podcasts.spec.ts
Les tests couvrent la création, l’aperçu, l’édition, la suppression, la lecture et la sélection d’une bibliothèque. L’ancien fichier regroupant les tests de playlists et de podcasts est supprimé.
Scénarios des podcasts webapp/e2e/podcasts.spec.ts
Les tests couvrent l’abonnement, le téléchargement, la lecture, la reprise, les jaquettes, les signets retardés, la déconnexion et les contrôles selon les permissions.
Commentaires et état du lecteur
Layer / File(s)
Summary
Requête de maintenance et lecteur webapp/src/api/queries/maintenance.ts, webapp/src/player/player.tsx, webapp/src/player/queue-element.ts
Le commentaire de la requête est déplacé. Le lecteur retire queueLength et ajuste ou supprime des commentaires; les résumés ne signalent pas de changement du traitement de fin de lecture.
No merge-blocking issue is established. The Canvas upload ceiling is enforced; the unrun Playwright suites remain for normal CI validation.
Security architecture risk: Minimal · up to 2da94
The Canvas change replaces an effectively unlimited fallback with a finite limit without expanding upload permissions. The inspected configuration refactoring preserves existing settings, and the administration changes preserve submitted role values. No introduced or materially worsened security risk was identified.
Retained concerns
No architecture-level concerns identified.
Security review details
Security Blast Radius
observed — Canvas request bytes are extracted before authentication inside the handler, so unauthenticated callers can exercise the body-buffering boundary. That ordering predates this PR. For conversion-failing configurations, the change reduces the per-request ceiling to 4 MiB; it does not establish an aggregate concurrency limit.
Trust Boundaries and Controls
observed — The request ceiling is a resource control, not mutation authorization. Canvas placement retains downstream checks of authenticated identity, library membership, upload permission and accepts_canvas, including authoritative checks within the write transaction. The fallback does not grant access to another library or bypass those controls.
Resilience and Maintainability Implications
observed — The fallback does not introduce a new Canvas storage transition. Negative configured limits are rejected before staging; accepted requests retain the existing quota-checked transactional replacement path and cleanup behavior. Deletion remains independently authorized and is not disabled by closing a library to new canvases.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 53.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surv…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Title check
Le titre décrit plusieurs changements réels de la PR : rôles traduits, découpage des specs e2e, plafond canvas et lecteurs de configuration. Il est précis et relié au contenu, mais il est long (119 ca…
Description check
La description suit le modèle : résumé, liste des changements par domaine, notes sur les vérifications et points signalés. Il manque une référence explicite au format de la section Test plan (cases à …
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
Le titre décrit plusieurs changements réels de la PR : rôles traduits, découpage des specs e2e, plafond canvas et lecteurs de configuration. Il est précis et relié au contenu, mais il est long (119 caractères) et énumère plusieurs sujets au lieu d'un seul changement principal. Il reste donc partiellement adapté, sans être hors sujet.
Full details: Description check
Explanation
La description suit le modèle : résumé, liste des changements par domaine, notes sur les vérifications et points signalés. Il manque une référence explicite au format de la section Test plan (cases à cocher du modèle), et la section Notes n'est pas formatée selon le modèle. Les vérifications exécutées sont décrites en prose, ce qui couvre l'essentiel. La description est donc presque complète.
Full details: Docstring Coverage
Explanation
Docstring coverage is 53.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 15 functions across 24 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches: 💡 1
📝 Generate docstrings: 💡
Commit to this branch
Create a new PR
🧪 Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
Biome (2.5.15)
webapp/e2e/admin.spec.ts
Biome could not lint this file: nested root configuration. Check the repository's Biome configuration and plugins.
webapp/e2e/artist.spec.ts
Biome could not lint this file: nested root configuration. Check the repository's Biome configuration and plugins.
webapp/e2e/editorial-pages.spec.ts
Biome could not lint this file: nested root configuration. Check the repository's Biome configuration and plugins.
13 others
Comment @coderabbitai help to get the list of available commands.
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
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.
github-actions
Bot
added
ignore-for-release
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
The loose ends the backend (#351–#358) and webapp (#359–#365) restructurings noted but did not fix, because those pull requests only moved code. One commit per subject.
Web client
fix(webapp)). The account rows printed the API'sadmin/userand a hard-coded English "N libraries"; the membership panel printedowner/manager/listenerand offered them as the role picker's labels. They now go through the translations (admin.role*,admin.accountLibrarieswith its plural). English reads exactly as before; French readsadministrateur · 1 bibliothèque,propriétaire,auditeur,gestionnaire. The<option>values are unchanged, so what is sent is unchanged.refactor(webapp)): thequeueLengthref was written on every render and read nowhere — removed. The order effect's comment said the queue is compared "by identity and by prefix";continuationOfcompares entry by entry and its own comment says why a prefix test was dropped — the comment now says so.onEnd's "stay on the last track" explanation was written twice; the copy outside the handler is gone.filedTreeQuerydoc comment sat above the "Needs attention" block, two doc comments stacked on one export; it is back onfiledTreeQuery.e2e
smart-playlists-and-podcasts.spec.ts(1,102 lines) split by subject intosmart-playlists.spec.ts(234) andpodcasts.spec.ts(452), with the shared mock ine2e/fixtures/smart-playlists-and-podcasts.ts(389). Its state was eight module-levellets, which an importing spec cannot reassign; they are now the fields of onestateobject, asfixtures/state.tsdoes for the rest, reset to the same eight values before each test. The layout gate's allowlist is now empty.session,albums,library(...),expectNoWcagViolations,navigateWithin, the one-pixel PNG (PIXEL, seven copies) andsilentWav(30)(whose removed copy's comment said "one second"; it was thirty). Copies that differ (other defaults, other fields, inline accessibility checks with other options) are left as they are.names the roles and the library count in the reader's language.Proof that nothing was lost:
playwright test --listgave 508 tests before the split and 508 after, with the sorted titles identical; 510 with the new test (one test × two projects).Server
fix(app)).bootturnedconfig.canvas.max_bytes(ani64) into the route's body ceiling withunwrap_or(usize::MAX).Config::from_envrefuses a value that does not fit, but aConfigbuilt by hand never meets that check, and a negative value then meant no ceiling at all on the one route that takes a whole video in a body. It now goes throughcanvas_body_limitinapp/state.rs, besidechunk_body_limit, and falls back to the default 4 MiB. Unit testa_canvas_size_that_does_not_fit_keeps_a_finite_ceiling; inverted: with the fallback put back tousize::MAXit fails.config: seven settings where zero is a value (API_TOKEN_TTL_SECS,SCAN_INTERVAL_SECS,UNAVAILABLE_RETENTION_DAYS,PURGE_GRACE_HOURS,QUALITY_PARALLELISM,MIGRATION_BACKUPS,PODCAST_REFRESH_INTERVAL_SECS) each re-wrotevar → default → parse → "invalid NAME: error"by hand, which isparse_envword for word. They call it now; defaults and error messages are unchanged.cli: the module comment said the CLI was "used before the M4 web administration surface exists". It now says what it is for, and whyrestoreandkeyrun without theAppState.CI
findmet no file and they passed having measured nothing. Inverted for each: fromwebapp/, a 601-linesrc/libfile passes the old webapp gate and fails the new one; fromsrc/, an 801-line file passes the old Rust gate and fails the new one.Checks run
cargo fmt --check,scripts/check-rust-layout.sh,clippy -D warnings,cargo doc -D warnings,cargo test --all-features: 667 passed (666 + the canvas test).webapp/:biome check(the same 2 warnings as main),bun run typecheck(src, node configs and e2e),bun run test327 passed,bun run build,scripts/check-webapp-layout.sh.Noticed, not changed
workspace-pages.spec.tsset the CSRF cookie forhttp://127.0.0.1:4173, ignoringbaseURL, so underWAVEFLOW_E2E_PORTit would go to the wrong origin. Kept as it was: changing it changes behaviour.workspace-pages.spec.tshas its ownEpisodetype andchannelOf, identical to the podcast fixture's; sharing them would make it import from the podcasts fixture.aria-labelreadsRôle: guestin French, without the space French puts before a colon.Summary by CodeRabbit
Nouvelles fonctionnalités
Corrections
Documentation