fix(pybuilder): recognize the Encodable marker on a trait member

[aglinxinyuan]

What changes were proposed in this PR?

EncodableInspector.safeAccessed hopped from an accessor symbol to accessed unconditionally. A trait member has no backing field of its own, so its accessor's accessed is NoSymbol — and that is exactly where scalac leaves the marker for a trait val/var, since @EncodableStringAnnotation targets METHOD as well as FIELD. The hop threw the annotation away, and the failure direction is the dangerous one:

Before:  trait T { @EncodableStringAnnotation val s: String }
         pyb"print(${t.s})"  ->  PyLiteralStringRenderer  ->  emitted raw, no diagnostic
After:                       ->  EncodableStringRenderer  ->  decode(...) at run time

Nothing warned about it at either end. BoundaryValidator.validateCompileTime runs only for arguments the inspector classified Encodable (PythonTemplateBuilder.scala:358), so a trait member spliced into an unsafe position skipped the compile-time boundary check as well.

The hop is now conditional — fall back to the accessor when there is no field to hop to:

case accessor: TermSymbol if accessor.isAccessor =>
  val field = accessor.accessed
  if (field == null || field == NoSymbol) accessor else field
case _ => sym

Measured one shape per row, with a throwaway probe macro that dumped the symbol and type state at expansion time (@Enc abbreviates @EncodableStringAnnotation):

Declaration	accessed	Marker sits on	Before	After
trait T { @Enc val s: String }	NoSymbol	the accessor	raw	encoded
trait T { @Enc val s: String = "x" }	NoSymbol	the accessor	raw	encoded
trait T { @Enc var s: String = "x" }	NoSymbol	the accessor	raw	encoded
class C { @Enc val s: String = "x" }	the field	the field	encoded	unchanged
case class H(@Enc ui: String)	the field	the ctor param	raw	unchanged
abstract class C { @Enc val s: String }	NoSymbol	—	raw	raw

Two rows deliberately do not move. The case-class one is the documented meta-annotation rule: without a meta-annotation the marker stays on the constructor parameter, and @(EncodableStringAnnotation @field) is the shape that reaches the field. The abstract-class one is out of reach from here — scalac keeps the marker on neither the accessor (its annotations is empty once its info is forced) nor any field, so the macro never sees it. Type position works there, val s: String @EncodableStringAnnotation, which is what the EncodableString alias expands to anyway; the spec asserts that working shape rather than cementing the hole with a negative test.

safeAccessed's second case, case methodAccessor: MethodSymbol if methodAccessor.isAccessor, goes at the same time. MethodSymbol is a subtype of TermSymbol in scala-reflect, so anything that reached it had already matched the first case: it could never fire, and it was not a trait path in disguise.

Any related issues, documentation, discussions?

Follows #7864, which added the probe macro these tests use and deliberately left this path unpinned so that fixing it would not have to fight a test that had cemented it. That PR has since merged, and this branch is rebased onto it, so the diff here is the fix alone.

#7864 also recorded methodReturnHasAnn as unpinnable without a production seam. That turned out to be wrong for a reason worth writing down; it is pinned here, and the stale comment is corrected.

How was this PR tested?

Five tests added to EncodableInspectorSpec, all driven through #7864's probe macro so they read the classifier's answers directly instead of inferring them from compile-error text:

Test	Pins
a trait's marked val is Encodable even though its accessor has no backing field	the defect itself, over three fixtures: abstract val, concrete val, var
a marked trait val is lowered to an EncodableStringRenderer	what the classification is for — wrapArg must reach priority 2, not the raw-literal default
an unmarked trait val stays a plain literal	the fallback reads the accessor's own annotations, and reads them selectively: a second fixture puts a foreign @(ZzAnnG @getter) annotation there
an abstract class's abstract val needs the marker in type position	the working shape for the row the fallback cannot reach
a def whose inline-annotated result type is stripped at the call site is Encodable	methodReturnHasAnn

Written before the fix. Mutation testing on the touched code — 4 mutants, 4 killed:

Mutant	Killed by
revert safeAccessed to the unconditional hop (the defect)	the two trait-member tests
never hop to the field, case accessor ... => accessor	the pre-existing @(EncodableStringAnnotation @field) test
symbol scan annotations.exists(annIsEncodableString) -> annotations.nonEmpty	an unmarked trait val stays a plain literal
case m: MethodSymbol => typeHasEncodableString(...) -> case _: MethodSymbol => false	an abstract class's abstract val ... and a def whose inline-annotated result type ...

The last row is the gap #7864 recorded. The trait-member fixture is not what closes it — a trait val is detected through the symbol path, not the signature path. A different measurement is: EncodableString is a type alias, so on def ui: EncodableString the call-site tree's own type dealiases back to the annotated type and the last disjunct answers true on its own, which is why the arm looked untestable. An inline def ui: String @EncodableStringAnnotation behaves differently — scalac strips the annotation off the call-site tree, leaving m.typeSignature.finalResultType the only place the marker survives.

sbt "PyBuilder/clean" "PyBuilder/test"

Tests: succeeded 189, failed 0 across 5 suites, 184 before.

sbt "PyBuilder/scalafmtCheckAll" "PyBuilder/scalafixAll --check"

Clean.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 5)

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. release/* labels drive the post-merge backport, so add or remove one to change where this fix lands.

Release branch	Analysis
✅ release/v1.2	Already labeled — this fix is queued to backport here.

Auto-label run.

[github-actions]

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

• Contributors with relevant context: @Yicong-Huang
  You can notify them by mentioning @Yicong-Huang in a comment.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.37%. Comparing base (4786d6c) to head (94104b1).

Files with missing lines	Patch %	Lines
...he/texera/amber/pybuilder/EncodableInspector.scala	50.00%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #7873      +/-   ##
============================================
+ Coverage     92.36%   92.37%   +0.01%     
- Complexity     4583     4587       +4     
============================================
  Files          1173     1173              
  Lines         47347    47348       +1     
  Branches       5306     5306              
============================================
+ Hits          43733    43740       +7     
+ Misses         2018     2017       -1     
+ Partials       1596     1591       -5     

Flag	Coverage Δ		*Carryforward flag
access-control-service	81.00% <ø> (ø)
agent-service	98.62% <ø> (ø)		Carriedforward from 4786d6c
amber	88.97% <50.00%> (+0.03%)	⬆️
computing-unit-managing-service	73.67% <ø> (ø)
config-service	86.73% <ø> (ø)
file-service	82.59% <ø> (ø)
frontend	94.22% <ø> (ø)		Carriedforward from 4786d6c
notebook-migration-service	79.13% <ø> (ø)
pyamber	97.57% <ø> (ø)		Carriedforward from 4786d6c
workflow-compiling-service	77.19% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

⚠️ Benchmark changes need a look

🟢 2 better · 🔴 3 worse · ⚪ 10 noise (<±5%) · 0 without baseline

Compared against main 4786d6c benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

	config	throughput	MB/s	latency	max Δ latest / 7d
🔴	bs=10 sw=10 sl=64	431	0.263	22,785/30,566/30,566 us	🟢 -9.6% / 🔴 +113.2%
⚪	bs=100 sw=10 sl=64	954	0.582	103,809/128,457/128,457 us	⚪ within ±5% / 🔴 +29.2%
🔴	bs=1000 sw=10 sl=64	1,097	0.67	906,139/1,029,116/1,029,116 us	🔴 +11.6% / 🔴 +7.3%

Baseline details

Latest main 4786d6c from same runner

config	metric	PR	latest main	7d avg	Δ latest	Δ 7d
bs=10 sw=10 sl=64	throughput	431 tuples/sec	446 tuples/sec	833.1 tuples/sec	-3.4%	-48.3%
bs=10 sw=10 sl=64	MB/s	0.263 MB/s	0.272 MB/s	0.508 MB/s	-3.3%	-48.3%
bs=10 sw=10 sl=64	p50	22,785 us	21,691 us	11,860 us	+5.0%	+92.1%
bs=10 sw=10 sl=64	p95	30,566 us	33,809 us	14,334 us	-9.6%	+113.2%
bs=10 sw=10 sl=64	p99	30,566 us	33,809 us	18,194 us	-9.6%	+68.0%
bs=100 sw=10 sl=64	throughput	954 tuples/sec	978 tuples/sec	1,085 tuples/sec	-2.5%	-12.1%
bs=100 sw=10 sl=64	MB/s	0.582 MB/s	0.597 MB/s	0.662 MB/s	-2.5%	-12.1%
bs=100 sw=10 sl=64	p50	103,809 us	102,870 us	92,973 us	+0.9%	+11.7%
bs=100 sw=10 sl=64	p95	128,457 us	132,445 us	99,460 us	-3.0%	+29.2%
bs=100 sw=10 sl=64	p99	128,457 us	132,445 us	107,737 us	-3.0%	+19.2%
bs=1000 sw=10 sl=64	throughput	1,097 tuples/sec	1,131 tuples/sec	1,111 tuples/sec	-3.0%	-1.3%
bs=1000 sw=10 sl=64	MB/s	0.67 MB/s	0.691 MB/s	0.678 MB/s	-3.0%	-1.2%
bs=1000 sw=10 sl=64	p50	906,139 us	885,052 us	915,816 us	+2.4%	-1.1%
bs=1000 sw=10 sl=64	p95	1,029,116 us	922,464 us	959,066 us	+11.6%	+7.3%
bs=1000 sw=10 sl=64	p99	1,029,116 us	922,464 us	993,644 us	+11.6%	+3.6%

Raw CSV

config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,464.05,200,128000,431,0.263,22785.10,30566.19,30566.19
1,100,10,64,20,2097.26,2000,1280000,954,0.582,103809.39,128456.90,128456.90
2,1000,10,64,20,18231.60,20000,12800000,1097,0.670,906139.19,1029115.85,1029115.85

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Yicong-Huang]

This PR conflicts with the base branch.