Skip to content

feat: add source field to ServerNotification.Log for selective BSP lo… - #97

Merged
sake92 merged 1 commit into
mainfrom
feat/log-source-tagging
Jun 17, 2026
Merged

feat: add source field to ServerNotification.Log for selective BSP lo…#97
sake92 merged 1 commit into
mainfrom
feat/log-source-tagging

Conversation

@sake92

@sake92 sake92 commented Jun 17, 2026

Copy link
Copy Markdown
Owner

…g forwarding

Add optional 'source: Option[String]' field to ServerNotification.Log, allowing log producers to tag their messages with a subsystem identifier (e.g. 'zinc', 'coursier', 'scalajs', 'scalanative'). The BSP callback now filters log forwarding by source — suppressing zinc compile chatter while forwarding non-zinc errors and warnings to the client.

This replaces the coarse isCompileTask boolean for log filtering, which didn't scale when non-compile BSP requests triggered transitive compilations.

Summary by CodeRabbit

  • New Features

    • Logs now include source information to identify their origin (coursier, ScalaJS, Scala Native, Zinc).
    • Log messages are formatted with source prefixes for better clarity.
  • Bug Fixes

    • Warning notifications are now forwarded to BSP client alongside error notifications.

…g forwarding

Add optional 'source: Option[String]' field to ServerNotification.Log, allowing
log producers to tag their messages with a subsystem identifier (e.g. 'zinc',
'coursier', 'scalajs', 'scalanative'). The BSP callback now filters log
forwarding by source — suppressing zinc compile chatter while forwarding
non-zinc errors and warnings to the client.

This replaces the coarse isCompileTask boolean for log filtering, which didn't
scale when non-compile BSP requests triggered transitive compilations.
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0136152b-778d-4932-9b17-6932e85f8eab

📥 Commits

Reviewing files that changed from the base of the PR and between 7b80e54 and dc5235e.

📒 Files selected for processing (13)
  • plugin-api/src/ba/sake/deder/ServerNotification.scala
  • plugin-api/src/ba/sake/deder/ServerNotificationsLogger.scala
  • server/src/ba/sake/deder/bsp/DederBspServer.scala
  • server/src/ba/sake/deder/deps/DependencyResolver.scala
  • server/src/ba/sake/deder/scalajs/DederScalaJsLogger.scala
  • server/src/ba/sake/deder/scalajs/ScalaJsLinker.scala
  • server/src/ba/sake/deder/scalajs/ScalaJsTestRunner.scala
  • server/src/ba/sake/deder/scalanative/DederScalaNativeLogger.scala
  • server/src/ba/sake/deder/scalanative/ScalaNativeLinker.scala
  • server/src/ba/sake/deder/scalanative/ScalaNativeTestRunner.scala
  • server/src/ba/sake/deder/zinc/DederZincLogger.scala
  • server/src/ba/sake/deder/zinc/ZincCompiler.scala
  • server/test/src/ba/sake/deder/testing/TeePrintStreamSuite.scala

📝 Walkthrough

Walkthrough

Adds an optional source: Option[String] field to ServerNotification.Log and its companion helper methods. All subsystem loggers (zinc, scalajs, scalanative, coursier) now tag their emitted notifications with their respective source strings. ServerNotificationsLogger formats the prefix using source with moduleId as fallback. DederBspServer widens BSP client forwarding to include WARNING in addition to ERROR.

Changes

source field propagation through notification pipeline

Layer / File(s) Summary
ServerNotification.Log data shape and helper methods
plugin-api/src/ba/sake/deder/ServerNotification.scala
Adds source: Option[String] = None to the Log enum case and updates all companion helpers (log, logError, logWarning, logInfo, logDebug, logTrace) to accept and forward it.
Log prefix formatting and BSP forwarding
plugin-api/src/ba/sake/deder/ServerNotificationsLogger.scala, server/src/ba/sake/deder/bsp/DederBspServer.scala
ServerNotificationsLogger builds a prefix from source (fallback to moduleId) and formats messages as [$prefix] $message. DederBspServer now forwards both ERROR and WARNING non-zinc logs to the BSP client, removing the previous ERROR-only and !isCompileTask guard.
Source tagging in subsystem loggers and linkers
server/src/ba/sake/deder/zinc/DederZincLogger.scala, server/src/ba/sake/deder/zinc/ZincCompiler.scala, server/src/ba/sake/deder/scalajs/DederScalaJsLogger.scala, server/src/ba/sake/deder/scalajs/ScalaJsLinker.scala, server/src/ba/sake/deder/scalajs/ScalaJsTestRunner.scala, server/src/ba/sake/deder/scalanative/DederScalaNativeLogger.scala, server/src/ba/sake/deder/scalanative/ScalaNativeLinker.scala, server/src/ba/sake/deder/scalanative/ScalaNativeTestRunner.scala, server/src/ba/sake/deder/deps/DependencyResolver.scala
Each subsystem passes its fixed source string ("zinc", "scalajs", "scalanative", "coursier") to all ServerNotification.log* call sites.
Test pattern match arity updates
server/test/src/ba/sake/deder/testing/TeePrintStreamSuite.scala
Updates all ServerNotification.Log pattern matches from 4-argument to 5-argument form across the five TeePrintStreamSuite test cases.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 A new field hops in, source by name,
Each logger now labels who's playing the game.
Zinc stamps its messages, scalajs too,
Scalanative and coursier join the queue!
The BSP now whispers both warnings and woes,
And tests count to five — that's how the story goes. 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: adding a source field to ServerNotification.Log, though it is truncated and incomplete.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/log-source-tagging

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sake92
sake92 merged commit 1519305 into main Jun 17, 2026
2 of 4 checks passed
@sake92
sake92 deleted the feat/log-source-tagging branch June 17, 2026 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant