refactor(semconv): cutover to Weaver + remove ~30k LoC dead code

[ANcpLua]

Summary

Net −22,005 LoC removed (+942 / −22,947). Zero-caller dead code deleted plus Weaver migration scaffolded on disk for a later focused session.

What's gone

Three generated files with zero callers in src/ (verified via namespace-qualified grep):

File	LoC	Why it existed
src/qyl.instrumentation/Instrumentation/SemanticConventions.g.cs	7553	Pipeline 2 C# const classes — never imported
src/qyl.instrumentation/Instrumentation/SemanticConventions.Utf8.g.cs	7555	Pipeline 2 UTF-8 byte literals — never imported
src/qyl.collector/Ingestion/OtlpAttributes.Utf8.g.cs	6923	Orphan from a pre-collector split, not emitted by any current generator

Plus ~125 LoC in eng/semconv/generate-semconv.ts (the generateCSharp and generateCSharpUtf8 functions + their callers) that emitted into those paths.

What broke (and got fixed)

ActivityExceptionTelemetry.cs used five class-member references from SemanticConventions.g.cs (ErrorTypeAttributes.Type, ExceptionTypeAttributes.Type, etc.). Each is a constant semconv key on a stable OTel section. Inlined as private const string at the top of the file — five lines, no runtime change.

OtlpAttributes.cs (different from the .Utf8.g.cs shadow) was initially deleted too and restored when the build failed: despite the misleading name, that file defines the OTLP wire-protocol record types (OtlpResourceLogs, OtlpAnyValue, etc.) consumed by OtlpConverter.cs. Architecture doc had flagged this as O-2 deletable; that call was wrong — kept for now.

Weaver migration scaffold

Not yet wired into nuke Generate. What's on disk:

• eng/semconv/registry-qyl/manifest.yaml — qyl's extension registry stub + include-prefix list
• eng/semconv/templates/registry/qyl/weaver.yaml — Weaver template config, params centralize the include-prefix list
• eng/semconv/templates/registry/qyl/semconv.ts.j2 — proof-of-pipeline Jinja template; runs against upstream v1.40.0 and emits a TS file with the expected export const shape (filtering by params.include_prefixes, verified locally against the Weaver CLI)

Remaining templates (C# facades, TypeSpec, DuckDB SQL) are the follow-up work. Old generate-semconv.ts stays as-is and keeps producing all three live outputs (TS, TSP, SQL, facades) until the Weaver template set is complete.

Test plan

• dotnet build qyl.slnx --tl:off — 0 errors, 13 warnings (unchanged from main)
• npx tsx eng/semconv/generate-semconv.ts — all three remaining outputs regenerate cleanly, facades emit
• Weaver scaffold: weaver registry generate against upstream v1.40.0 + qyl/weaver.yaml produces semconv.ts with filtered namespace groups (proof of pipeline)
• CI green on push

Not in scope

• O-3 Qyl.Contracts.Models.SpanRecord.cs — Qyl.Contracts.Models is actively imported by ~6 qyl.mcp tool files; needs a focused caller audit before deletion.
• O-4 ToolManifestEmitter duplication — ToolManifestAnalyzer IS consumed by ServiceDefaultsSourceGenerator, not safe as a drive-by.
• Remaining Weaver templates (C#/TypeSpec/DuckDB/facades) — next session.

🤖 Generated with Claude Code

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This pull request migrates semantic conventions generation from a TypeScript-based generator to a Weaver-based workflow. The standalone Node.js script eng/semconv/generate-semconv.ts is removed along with its npm infrastructure, while new bash bootstrapping and execution scripts orchestrate the Weaver CLI against an upstream semantic-conventions registry. Configuration moves to Jinja2 templates and a YAML file. Generated outputs are regenerated: TypeScript attribute constants are restructured, SQL promoted columns are recalculated with updated type mappings, and previously auto-generated facade classes are converted to hand-maintained. The build pipeline is refactored to invoke bash scripts instead of npm tasks, and the generated TypeSpec bridge file is removed as it became unused.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title follows conventional commits format with a 'refactor' type, a clear '(semconv)' scope, and a descriptive subject summarizing the main change (Weaver cutover + dead code removal). It is 64 characters and directly reflects the changeset.
Description check	✅ Passed	The description is directly related to the changeset, detailing what was removed, what was fixed, and what scaffolding was added. It provides context and rationale for the changes across all modified files.
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.}

---

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

[Copilot]

Pull request overview

Removes unused generated C# semantic-convention outputs and updates the semconv generation tooling, while adding an initial Weaver-based scaffold for a future migration.

Changes:

• Deleted large unused C# semconv generated output and removed the C# generators/flags from eng/semconv/generate-semconv.ts.
• Inlined the handful of semconv keys needed by instrumentation exception telemetry.
• Added Weaver registry/template scaffolding and regenerated/normalized the TS semconv output formatting.

Reviewed changes

Copilot reviewed 5 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/qyl.instrumentation/Instrumentation/SemanticConventions.g.cs	Deletes unused generated C# semconv constants file.
src/qyl.instrumentation/Instrumentation/ActivityExceptionTelemetry.cs	Replaces references to deleted generated constants with inlined literal keys.
src/qyl.dashboard/src/lib/semconv.ts	Regenerated TS semconv output; formatting changes in enum object blocks.
eng/semconv/templates/registry/qyl/weaver.yaml	Adds Weaver template config + include-prefix filtering parameters.
eng/semconv/templates/registry/qyl/semconv.ts.j2	Adds initial Weaver TS template scaffold for semconv attributes.
eng/semconv/registry-qyl/manifest.yaml	Adds qyl registry manifest stub (prefix include list).
eng/semconv/generate-semconv.ts	Removes C# generation code paths and related CLI flags; updates outputs config and comments.
.gitignore	Ignores local Weaver/tool scratch directories.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

include_prefixes is duplicated here and in eng/semconv/registry-qyl/manifest.yaml. Keeping two authoritative lists increases the chance they drift and templates silently generate different subsets than intended.

Suggested fix: pick a single source of truth for the prefix list (either only params.include_prefixes in this Weaver config, or only the manifest + pass it into templates) and remove the other copy.

[Copilot]

The header hardcodes semconv version v1.40.0 even though params.semconv_version exists in weaver.yaml. This can easily become inconsistent when the version is bumped.

Suggested fix: interpolate the version from params.semconv_version (and consider matching the repo/package naming used by the existing generator headers for consistency).

Suggested change

	// Generated from open-telemetry/semantic-conventions v1.40.0 via Weaver
	// Generated from open-telemetry/semantic-conventions {{ params.semconv_version }} via Weaver

[Copilot]

--cs-only/--utf8-only flags were removed, but they are still referenced by eng/semconv/package.json scripts. With the current arg parsing, passing these unknown flags makes generateAll true, so running those scripts will unexpectedly regenerate all outputs (TS/TypeSpec/SQL/protocol) instead of only C#.

Suggested fix: either keep --cs-only/--utf8-only as supported (even if they become no-ops that error/warn), or update the npm scripts (and any docs/CI invocations) to remove them so behavior matches intent.

Suggested change

	const protocolOnly = args.includes("--protocol-only");
	const protocolOnly = args.includes("--protocol-only");
	const csOnly = args.includes("--cs-only");
	const utf8Only = args.includes("--utf8-only");
	if (csOnly || utf8Only) {
	const legacyFlags = [
	csOnly ? "--cs-only" : undefined,
	utf8Only ? "--utf8-only" : undefined,
	].filter((flag): flag is string => Boolean(flag));
	console.error(
	`Unsupported legacy flag(s): ${legacyFlags.join(", ")}. ` +
	"These flags were removed and must not be passed to this script."
	);
	process.exitCode = 1;
	return;
	}

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

eng/semconv/generate-semconv.ts (1)

5-14: ⚠️ Potential issue | 🟡 Minor

Docstring references removed C# generation.

Line 11 documents npm run generate:cs # C# only but this flag was removed. Update the usage block to reflect the actual supported flags.

Proposed fix

 * Usage:
 *   npm run generate           # Generate all outputs
 *   npm run generate:ts        # TypeScript only
-*   npm run generate:cs        # C# only
 *   npm run generate:tsp       # TypeSpec only
 *   npm run generate:sql       # DuckDB only
+*   npm run generate:protocol  # Protocol facades only

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@eng/semconv/generate-semconv.ts` around lines 5 - 14, The top-of-file usage
comment in the generate-semconv.ts script still references "npm run generate:cs
# C# only" and lists C# as an output; remove that C# reference and update the
header/Usage block to reflect the actual supported targets (TypeScript,
TypeSpec, DuckDB) by deleting the "npm run generate:cs" line and removing C#
from the initial description so the Usage shows only npm run generate, npm run
generate:ts, npm run generate:tsp, and npm run generate:sql; modify the comment
block (the usage block at the top of generate-semconv.ts) accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@eng/semconv/generate-semconv.ts`:
- Around line 864-870: The package.json scripts still pass --cs-only and
--utf8-only but main() only checks tsOnly, tspOnly, sqlOnly and protocolOnly so
generateAll becomes true; update main() to also parse const csOnly =
args.includes("--cs-only") and const utf8Only = args.includes("--utf8-only") and
include them in the generateAll calculation (generateAll = !tsOnly && !tspOnly
&& !sqlOnly && !protocolOnly && !csOnly && !utf8Only) so the single-mode scripts
(referenced by the --cs-only and --utf8-only flags) behave as intended; adjust
any downstream branching that uses these flags (locations referencing
tsOnly/tspOnly/sqlOnly/protocolOnly) to handle csOnly and utf8Only consistently.

In `@eng/semconv/registry-qyl/manifest.yaml`:
- Around line 13-76: The include_prefixes array in manifest.yaml is a duplicate
of the allowlist in weaver.yaml; remove the redundant copy from the TS side and
update the TS generator to parse this manifest instead of using its hardcoded
array: modify the generator logic (where the hardcoded allowlist is defined/used
in the TypeScript generator module) to read and parse manifest.yaml
include_prefixes and use that list at runtime/build time, and delete the
duplicated array so include_prefixes in manifest.yaml becomes the single source
of truth.

In `@eng/semconv/templates/registry/qyl/semconv.ts.j2`:
- Around line 10-17: The template semconv.ts.j2 currently only emits attribute
key constants (export const {{ attr.name | screaming_snake_case }}) and omits
the corresponding enum/value objects like GenAiSystemValues and
HttpRequestMethodValues; update the template to also iterate each attribute's
values (e.g., group.attributes -> attr.values) and emit a matching exported
values object for any attribute with defined values (for example an exported
object named {{ attr.name | pascal_case }}Values or {{ attr.name }}Values
containing key/value pairs for each enum constant), ensuring the same naming
scheme used elsewhere so consumers that import GenAiSystemValues,
HttpRequestMethodValues, etc. will receive the generated enums; alternatively,
if enums are intentionally deferred, add a clear comment in semconv.ts.j2
indicating enum generation is disabled and document where/when they will be
produced.

In `@eng/semconv/templates/registry/qyl/weaver.yaml`:
- Around line 14-77: The include_prefixes list in registry/qyl/weaver.yaml is
duplicated across generate-semconv.ts (CONFIG.includePrefixes) and
registry-qyl/manifest.yaml; consolidate to a single canonical source (recommend
registry-qyl/manifest.yaml) and update generate-semconv.ts and the weaver
template to read that manifest at runtime instead of hardcoding
CONFIG.includePrefixes or embedding the list in weaver.yaml: change
generate-semconv.ts to load the manifest’s include_prefixes and export/use that
value, and update the weaver template to reference the manifest-provided
include_prefixes (or accept it as a templating variable) so only manifest.yaml
maintains the authoritative list.

In `@src/qyl.instrumentation/Instrumentation/ActivityExceptionTelemetry.cs`:
- Around line 10-14: ActivityExceptionTelemetry defines hardcoded semconv keys
(ErrorType, ExceptionType, ExceptionMessage, ExceptionStacktrace,
ExceptionEscaped) that duplicate the authoritative constants in GenAiAttributes;
remove these duplicate constants and replace their usages in
ActivityExceptionTelemetry with the corresponding GenAiAttributes.<ConstantName>
references (e.g. use GenAiAttributes.ErrorType, GenAiAttributes.ExceptionType,
GenAiAttributes.ExceptionMessage, GenAiAttributes.ExceptionStacktrace,
GenAiAttributes.ExceptionEscaped) so there is a single source of truth and no
cross-layer spec drift.

---

Outside diff comments:
In `@eng/semconv/generate-semconv.ts`:
- Around line 5-14: The top-of-file usage comment in the generate-semconv.ts
script still references "npm run generate:cs # C# only" and lists C# as an
output; remove that C# reference and update the header/Usage block to reflect
the actual supported targets (TypeScript, TypeSpec, DuckDB) by deleting the "npm
run generate:cs" line and removing C# from the initial description so the Usage
shows only npm run generate, npm run generate:ts, npm run generate:tsp, and npm
run generate:sql; modify the comment block (the usage block at the top of
generate-semconv.ts) accordingly.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ac0f764e-69de-4fb6-aa0a-5759bd061d24

📥 Commits

Reviewing files that changed from the base of the PR and between bf39265 and f5152e6.

⛔ Files ignored due to path filters (4)

• .gitignore is excluded by none and included by none
• src/qyl.collector/Ingestion/OtlpAttributes.Utf8.g.cs is excluded by !**/*.g.cs and included by src/**
• src/qyl.instrumentation/Instrumentation/SemanticConventions.Utf8.g.cs is excluded by !**/*.g.cs and included by src/**
• src/qyl.instrumentation/Instrumentation/SemanticConventions.g.cs is excluded by !**/*.g.cs and included by src/**

📒 Files selected for processing (6)

• eng/semconv/generate-semconv.ts
• eng/semconv/registry-qyl/manifest.yaml
• eng/semconv/templates/registry/qyl/semconv.ts.j2
• eng/semconv/templates/registry/qyl/weaver.yaml
• src/qyl.dashboard/src/lib/semconv.ts
• src/qyl.instrumentation/Instrumentation/ActivityExceptionTelemetry.cs

📜 Review details

⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: Agent
• GitHub Check: Analyze (csharp)

🧰 Additional context used

📓 Path-based instructions (4)

**/*.cs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.cs: New .cs files must be UTF-8 with BOM. Required for dotnet format to behave. .editorconfig declares charset = utf-8-bom under [*.cs].
Include copyright header // Copyright (c) 2025-2026 ancplua at the top of .cs files (personal repo; MAF uses the Microsoft header).
XML documentation is required on all public methods and classes. GenerateDocumentationFile=true is set in Directory.Build.props.
Use the Async suffix for any method returning Task / ValueTask — including test methods.
Declare private classes as sealed unless intentionally subclassed.
Use C# 14 with preview features enabled. File-scoped namespaces, primary constructors, required init properties, pattern matching, switch expressions over if-else.
No suppression of warnings. Forbidden: #pragma warning disable, [SuppressMessage], <NoWarn>. Exception: upstream sample repos demonstrating experimental APIs. qyl's WarningsAsErrors=CA1816;CA2012;CA2016 is already minimal — add more rules, never subtract.
Use IIncrementalGenerator only for generators, with ForAttributeWithMetadataName, value-equatable models, raw strings over SyntaxFactory. Never store ISymbol in models. Test generators via ANcpLua.Roslyn.Utilities test infrastructure.
Never use runtime reflection as a control mechanism, dynamic / ExpandoObject, blocking async (.Result / .Wait()), or any analyzer besides ANcpLua.Analyzers. Do not suppress null ! when the code can be rewritten.
Arrange / Act / Assert comments in test methods. Use the project's FakeChatClient (tests/qyl.collector.tests/Instrumentation/) for IChatClient doubles — do NOT hand-roll Moq<IChatClient>.
Wrap every IChatClient AND the AIAgent with the telemetry pipeline — both layers, not one. Use UseQylTelemetry (the qyl wrapper) on the chat client, not a hand-rolled UseFunctionInvocation().UseOpenTelemetry(...) chain.
When constructing IChatClient, set EnableSensitiveData = null to defer to `OTEL_INSTRUMENTATION_GE...

Files:

• src/qyl.instrumentation/Instrumentation/ActivityExceptionTelemetry.cs

src/**/*.cs

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.cs: Do not let MAF types leak into qyl.contracts. Do not let the dashboard call runtime code directly — it goes through the collector REST API. Dependency direction is one-way: contracts at bottom, generators separate, runtime depends on contracts + outputs.
Reference ~/Apex.AgenticEntityExtractor/ (canonical qyl consumer shape) when building new qyl services. Mirror the three-builder-interface + fluent-middleware pattern from: Agents/ExtractorAgentsBuilder.cs, Clients/ExtractorChatClientBuilder.cs, Workflows/ExtractorWorkflowBuilder.cs, Program.cs.

Files:

• src/qyl.instrumentation/Instrumentation/ActivityExceptionTelemetry.cs

---

⚙️ CodeRabbit configuration file

src/**/*.cs: C#/.NET codebase. Review for: idiomatic C# patterns, proper async/await (no sync-over-async, no fire-and-forget without justification), correct IDisposable/IAsyncDisposable, null safety, and adherence to existing patterns. Flag new public API surface. Check DI lifetime correctness (scoped vs singleton vs transient).
ARCHITECTURAL INVARIANTS — flag violations as blocking: - Every new injectable service must register OpenTelemetry instrumentation (ActivitySource or Meter). - Every new DuckDB write path must handle backpressure (bounded channel or semaphore). - No hardcoded connection strings, paths, or magic strings — use IOptions or IConfiguration. - No new dependencies on Sentry-specific types in core/ — Sentry is an optional backend, not the identity. - CancellationToken must be threaded through all async public methods.

Files:

• src/qyl.instrumentation/Instrumentation/ActivityExceptionTelemetry.cs

eng/**

⚙️ CodeRabbit configuration file

Build and deployment infrastructure. Review for: correct MSBuild property usage, Nuke build target dependencies, Docker multi-stage build efficiency, and CI/CD pipeline correctness. Flag hardcoded paths, secrets, or platform-specific assumptions.

Files:

• eng/semconv/templates/registry/qyl/weaver.yaml
• eng/semconv/templates/registry/qyl/semconv.ts.j2
• eng/semconv/generate-semconv.ts
• eng/semconv/registry-qyl/manifest.yaml

src/qyl.dashboard/**

⚙️ CodeRabbit configuration file

React/TypeScript dashboard. Review for: single-responsibility components, proper hook usage (no hooks in conditionals, correct dependency arrays), TypeScript strictness (no any unless justified with a comment), and basic accessibility. Uses Tailwind — flag inline styles or custom CSS that duplicates utility classes. Flag any direct fetch() calls that bypass the shared API client.

Files:

• src/qyl.dashboard/src/lib/semconv.ts

🧠 Learnings (9)

📓 Common learnings

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: src/qyl.mcp/CLAUDE.md:0-0
Timestamp: 2026-04-21T00:49:29.212Z
Learning: Applies to src/qyl.mcp/src/qyl.mcp.generators/**/*.cs : `src/qyl.mcp.generators/` must emit `QylToolManifest` with `ToolTypes[]`, `ToolDescriptors[]`, and `CreateTools()`

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: tests/qyl.mcp.generators.tests/CLAUDE.md:0-0
Timestamp: 2026-04-21T00:49:34.297Z
Learning: Applies to tests/qyl.mcp.generators.tests/**/*.cs : Use `System.Text.Json` instead of Newtonsoft in C# code

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: src/qyl.mcp/CLAUDE.md:0-0
Timestamp: 2026-04-21T00:49:29.212Z
Learning: Applies to src/qyl.mcp/src/qyl.mcp.generators/**/*.cs : Use `IndentedStringBuilder.BeginBlock()` pattern instead of `Indent()/Outdent()` (which are internal) in generator code

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: src/qyl.mcp/CLAUDE.md:0-0
Timestamp: 2026-04-21T00:49:29.212Z
Learning: Applies to src/qyl.mcp/**/*.cs : Use `System.Text.Json` instead of Newtonsoft for JSON serialization in C# code

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to **/*.cs : Compile-time wiring over runtime reflection: the generator owns DI registration, MCP tool registration, and capability catalogs. Use `[QylSkill]` + `[QylCapability]` attributes instead of hand-registering tools.

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to **/*.cs : Do NOT hand-add tools to DI, MCP registration, or skill catalogs. Use `[QylSkill(QylSkillKind.X)]` and `[QylCapability]` attributes instead — the generator handles registration from the attribute.

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to **/*.cs : Use `IIncrementalGenerator` only for generators, with `ForAttributeWithMetadataName`, value-equatable models, raw strings over `SyntaxFactory`. Never store `ISymbol` in models. Test generators via `ANcpLua.Roslyn.Utilities` test infrastructure.

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to **/*.cs : No suppression of warnings. Forbidden: `#pragma warning disable`, `[SuppressMessage]`, `<NoWarn>`. Exception: upstream sample repos demonstrating experimental APIs. qyl's `WarningsAsErrors=CA1816;CA2012;CA2016` is already minimal — add more rules, never subtract.

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to src/**/*.cs : Do not let MAF types leak into `qyl.contracts`. Do not let the dashboard call runtime code directly — it goes through the collector REST API. Dependency direction is one-way: contracts at bottom, generators separate, runtime depends on contracts + outputs.

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to src/qyl.mcp/**/*.cs : Declare tool class with `[McpServerToolType]` and `[QylSkill(QylSkillKind.X)]`. Declare tool methods with `[QylCapability("id", Starting|FollowUp)]`. Never hand-register tools — let the generator produce `RegisterTools()`, `RegisterServices()`, `Capabilities[]`, `ToolDescriptors[]`.

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to **/*.cs : Both IChatClient and AIAgent telemetry layers must live at the composition root, never scattered through call sites. No attribute-based agent tracing — `[AgentTraced]` was removed.

Applied to files:

• src/qyl.instrumentation/Instrumentation/ActivityExceptionTelemetry.cs

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to **/*.cs : Use standard OTel environment variables (OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_PROTOCOL, OTEL_SERVICE_NAME, ENABLE_INSTRUMENTATION) instead of qyl-invented ones. Prefer these over hardcoded config.

Applied to files:

• eng/semconv/templates/registry/qyl/weaver.yaml

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to **/*.g.ts : Never hand-edit *.g.ts files. Fix the generator input instead.

Applied to files:

• eng/semconv/templates/registry/qyl/semconv.ts.j2
• eng/semconv/generate-semconv.ts

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to **/*.g.cs : Never hand-edit *.g.cs files. Fix the generator input (TypeSpec model, attribute, routing table) instead.

Applied to files:

• eng/semconv/generate-semconv.ts

📚 Learning: 2026-04-21T00:49:29.212Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: src/qyl.mcp/CLAUDE.md:0-0
Timestamp: 2026-04-21T00:49:29.212Z
Learning: Applies to src/qyl.mcp/src/qyl.mcp.generators/**/*.cs : `src/qyl.mcp.generators/` must emit `QylToolManifest` with `ToolTypes[]`, `ToolDescriptors[]`, and `CreateTools()`

Applied to files:

• eng/semconv/generate-semconv.ts

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to **/*.cs : Use `IIncrementalGenerator` only for generators, with `ForAttributeWithMetadataName`, value-equatable models, raw strings over `SyntaxFactory`. Never store `ISymbol` in models. Test generators via `ANcpLua.Roslyn.Utilities` test infrastructure.

Applied to files:

• eng/semconv/generate-semconv.ts

📚 Learning: 2026-04-21T00:49:34.297Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: tests/qyl.mcp.generators.tests/CLAUDE.md:0-0
Timestamp: 2026-04-21T00:49:34.297Z
Learning: Applies to tests/qyl.mcp.generators.tests/**/*.cs : Use `System.Text.Json` instead of Newtonsoft in C# code

Applied to files:

• eng/semconv/generate-semconv.ts

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to src/**/*.cs : Do not let MAF types leak into `qyl.contracts`. Do not let the dashboard call runtime code directly — it goes through the collector REST API. Dependency direction is one-way: contracts at bottom, generators separate, runtime depends on contracts + outputs.

Applied to files:

• eng/semconv/generate-semconv.ts

🔇 Additional comments (1)

src/qyl.dashboard/src/lib/semconv.ts (1)

1-3: Formatting-only changes; file remains generated by legacy process.

The header confirms this file is still generated via npm run generate, not the new Weaver scaffold. The indentation normalization on *Values objects is benign.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Second copy of the allowlist—see weaver.yaml comment.

Same duplication concern applies. This manifest could serve as the single source if the TS generator parsed it instead of maintaining its own hardcoded array.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@eng/semconv/registry-qyl/manifest.yaml` around lines 13 - 76, The
include_prefixes array in manifest.yaml is a duplicate of the allowlist in
weaver.yaml; remove the redundant copy from the TS side and update the TS
generator to parse this manifest instead of using its hardcoded array: modify
the generator logic (where the hardcoded allowlist is defined/used in the
TypeScript generator module) to read and parse manifest.yaml include_prefixes
and use that list at runtime/build time, and delete the duplicated array so
include_prefixes in manifest.yaml becomes the single source of truth.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Template emits attribute keys only; enum values are missing.

The current semconv.ts includes *Values objects (e.g., GenAiSystemValues, HttpRequestMethodValues). This template only emits export const attribute keys. When wired up, the generated output will drop ~1100 lines of enum constants, breaking any consumers that import them.

Either extend the template to emit enum values or document that enum generation is intentionally deferred.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@eng/semconv/templates/registry/qyl/semconv.ts.j2` around lines 10 - 17, The
template semconv.ts.j2 currently only emits attribute key constants (export
const {{ attr.name | screaming_snake_case }}) and omits the corresponding
enum/value objects like GenAiSystemValues and HttpRequestMethodValues; update
the template to also iterate each attribute's values (e.g., group.attributes ->
attr.values) and emit a matching exported values object for any attribute with
defined values (for example an exported object named {{ attr.name | pascal_case
}}Values or {{ attr.name }}Values containing key/value pairs for each enum
constant), ensuring the same naming scheme used elsewhere so consumers that
import GenAiSystemValues, HttpRequestMethodValues, etc. will receive the
generated enums; alternatively, if enums are intentionally deferred, add a clear
comment in semconv.ts.j2 indicating enum generation is disabled and document
where/when they will be produced.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Triple-maintained allowlist creates spec drift risk.

This include_prefixes list is duplicated verbatim in:

1. eng/semconv/generate-semconv.ts (CONFIG.includePrefixes)
2. eng/semconv/registry-qyl/manifest.yaml
3. This file

Adding a new upstream prefix requires synchronized edits across all three. Consider a single canonical source (e.g., manifest.yaml) that the TS generator and Weaver templates both read at runtime.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@eng/semconv/templates/registry/qyl/weaver.yaml` around lines 14 - 77, The
include_prefixes list in registry/qyl/weaver.yaml is duplicated across
generate-semconv.ts (CONFIG.includePrefixes) and registry-qyl/manifest.yaml;
consolidate to a single canonical source (recommend registry-qyl/manifest.yaml)
and update generate-semconv.ts and the weaver template to read that manifest at
runtime instead of hardcoding CONFIG.includePrefixes or embedding the list in
weaver.yaml: change generate-semconv.ts to load the manifest’s include_prefixes
and export/use that value, and update the weaver template to reference the
manifest-provided include_prefixes (or accept it as a templating variable) so
only manifest.yaml maintains the authoritative list.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Eliminate duplicated semconv key ownership to prevent cross-layer spec drift.

ActivityExceptionTelemetry now hardcodes keys already defined in src/qyl.contracts/Attributes/GenAiAttributes.g.cs, while other instrumentation paths still use GenAiAttributes. This creates two authorities for the same attributes (error.type, exception.type, etc.) and can silently diverge.

Proposed refactor

-    private const string ErrorType = "error.type";
-    private const string ExceptionType = "exception.type";
-    private const string ExceptionMessage = "exception.message";
-    private const string ExceptionStacktrace = "exception.stacktrace";
+    private const string ExceptionEscaped = "exception.escaped";
@@
-        activity.SetTag(ErrorType, ResolveErrorType(exception, errorType));
+        activity.SetTag(GenAiAttributes.ErrorType, ResolveErrorType(exception, errorType));
@@
-            { ExceptionType, exception.GetType().FullName },
-            { ExceptionMessage, exception.Message },
-            { ExceptionStacktrace, exception.ToString() },
+            { GenAiAttributes.ExceptionType, exception.GetType().FullName },
+            { GenAiAttributes.ExceptionMessage, exception.Message },
+            { GenAiAttributes.ExceptionStacktrace, exception.ToString() },
             { ExceptionEscaped, escaped }

Also applies to: 38-38, 44-47

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/qyl.instrumentation/Instrumentation/ActivityExceptionTelemetry.cs` around
lines 10 - 14, ActivityExceptionTelemetry defines hardcoded semconv keys
(ErrorType, ExceptionType, ExceptionMessage, ExceptionStacktrace,
ExceptionEscaped) that duplicate the authoritative constants in GenAiAttributes;
remove these duplicate constants and replace their usages in
ActivityExceptionTelemetry with the corresponding GenAiAttributes.<ConstantName>
references (e.g. use GenAiAttributes.ErrorType, GenAiAttributes.ExceptionType,
GenAiAttributes.ExceptionMessage, GenAiAttributes.ExceptionStacktrace,
GenAiAttributes.ExceptionEscaped) so there is a single source of truth and no
cross-layer spec drift.

[coderabbitai]

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@eng/semconv/bootstrap-weaver.sh`:
- Line 40: Echo line uses unquoted command substitution which can lead to word
splitting; update the echo to quote the command substitution and variable
expansions so the binary path and the output of the version command are
preserved as single fields. Specifically, change the echo that references
WEAVER_DIR, WEAVER_ARCH and the weaver --version invocation so the binary path
and the "$(${WEAVER_DIR}/weaver-${WEAVER_ARCH}/weaver --version)" are quoted
(e.g., quote "${WEAVER_DIR}/weaver-${WEAVER_ARCH}/weaver" and "$(...)" around
the --version call).
- Around line 16-21: The case block that sets WEAVER_ARCH based only emits
apple-darwin targets and fails on Linux; update the logic that inspects UNAME_M
(and optionally uname -s) so x86_64 on Linux sets
WEAVER_ARCH="x86_64-unknown-linux-gnu" while preserving the existing
"x86_64-apple-darwin" and "aarch64-apple-darwin" mappings for macOS; adjust the
case or add a nested OS check around the UNAME_M branches (refer to the UNAME_M
variable and the WEAVER_ARCH assignment) so Linux CI on x86_64 gets the correct
weaver binary name.

In `@eng/semconv/run-weaver.sh`:
- Line 14: The WEAVER_BIN assignment currently hardcodes "aarch64-apple-darwin"
which breaks portability; change the assignment in eng/semconv/run-weaver.sh so
WEAVER_BIN is computed using the same architecture/OS detection used by
bootstrap-weaver.sh (or source/bootstrap that script) instead of the hardcoded
string, e.g., derive an ARCH/PLATFORM variable and build the path from REPO_ROOT
and that variable, and add a clear existence check that fails with an error if
the computed WEAVER_BIN does not exist; update references to WEAVER_BIN
accordingly.

In `@eng/semconv/templates/registry/qyl/promoted_columns.g.sql.j2`:
- Around line 11-12: The suffix-matching currently uses
attr.name.endswith('_tokens') etc. against dotted attribute names, so it never
matches; change the logic to perform suffix checks on the underscore-converted
attribute name (e.g., set a local variable like attr_underscore = attr.name |
replace('.', '_') or call attr.name.replace('.', '_') and then use
attr_underscore.endswith(...) against bigint_suffixes and double_suffixes),
updating the checks where attr.name.endswith is used (including the occurrences
referenced around bigint_suffixes/double_suffixes and the similar checks at the
other mentioned lines) so attributes like azure.cosmosdb.request.body.size
become azure_cosmosdb_request_body_size and are typed correctly.

In `@eng/semconv/templates/registry/qyl/weaver.yaml`:
- Line 10: The hardcoded semconv_version ("1.40.0") in weaver.yaml risks drift;
centralize the version so both semconv_version and the bootstrap script (which
uses SEMCONV_TAG) read a single source. Update semconv_version to reference a
shared variable (e.g., load from a .env or manifest) and change
bootstrap-weaver.sh to read SEMCONV_TAG from the same source (or export it from
the manifest loader), ensuring the symbols semconv_version and SEMCONV_TAG are
derived from one canonical setting so upgrades only require one edit.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: fbe10510-2af4-4b93-8547-4cec9ca615c9

📥 Commits

Reviewing files that changed from the base of the PR and between f5152e6 and 4a907d4.

📒 Files selected for processing (5)

• eng/semconv/bootstrap-weaver.sh
• eng/semconv/run-weaver.sh
• eng/semconv/templates/registry/qyl/promoted_columns.g.sql.j2
• eng/semconv/templates/registry/qyl/semconv.ts.j2
• eng/semconv/templates/registry/qyl/weaver.yaml

📜 Review details

⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Analyze (csharp)

🧰 Additional context used

📓 Path-based instructions (1)

eng/**

⚙️ CodeRabbit configuration file

Build and deployment infrastructure. Review for: correct MSBuild property usage, Nuke build target dependencies, Docker multi-stage build efficiency, and CI/CD pipeline correctness. Flag hardcoded paths, secrets, or platform-specific assumptions.

Files:

• eng/semconv/templates/registry/qyl/weaver.yaml
• eng/semconv/templates/registry/qyl/semconv.ts.j2
• eng/semconv/run-weaver.sh
• eng/semconv/bootstrap-weaver.sh
• eng/semconv/templates/registry/qyl/promoted_columns.g.sql.j2

🧠 Learnings (2)

📓 Common learnings

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: src/qyl.mcp/CLAUDE.md:0-0
Timestamp: 2026-04-21T00:49:29.212Z
Learning: Applies to src/qyl.mcp/src/qyl.mcp.generators/**/*.cs : `src/qyl.mcp.generators/` must emit `QylToolManifest` with `ToolTypes[]`, `ToolDescriptors[]`, and `CreateTools()`

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: tests/qyl.mcp.generators.tests/CLAUDE.md:0-0
Timestamp: 2026-04-21T00:49:34.297Z
Learning: Applies to tests/qyl.mcp.generators.tests/**/*.cs : Use `System.Text.Json` instead of Newtonsoft in C# code

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to **/*.cs : Compile-time wiring over runtime reflection: the generator owns DI registration, MCP tool registration, and capability catalogs. Use `[QylSkill]` + `[QylCapability]` attributes instead of hand-registering tools.

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to src/**/*.cs : Do not let MAF types leak into `qyl.contracts`. Do not let the dashboard call runtime code directly — it goes through the collector REST API. Dependency direction is one-way: contracts at bottom, generators separate, runtime depends on contracts + outputs.

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: src/qyl.mcp/CLAUDE.md:0-0
Timestamp: 2026-04-21T00:49:29.212Z
Learning: Applies to src/qyl.mcp/src/qyl.mcp.generators/**/*.cs : Use `IndentedStringBuilder.BeginBlock()` pattern instead of `Indent()/Outdent()` (which are internal) in generator code

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: src/qyl.mcp/CLAUDE.md:0-0
Timestamp: 2026-04-21T00:49:29.212Z
Learning: Applies to src/qyl.mcp/**/*.cs : Use `System.Text.Json` instead of Newtonsoft for JSON serialization in C# code

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to **/*.cs : Do NOT hand-add tools to DI, MCP registration, or skill catalogs. Use `[QylSkill(QylSkillKind.X)]` and `[QylCapability]` attributes instead — the generator handles registration from the attribute.

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to **/*.cs : Use standard OTel environment variables (OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_PROTOCOL, OTEL_SERVICE_NAME, ENABLE_INSTRUMENTATION) instead of qyl-invented ones. Prefer these over hardcoded config.

Applied to files:

• eng/semconv/templates/registry/qyl/weaver.yaml

🪛 Shellcheck (0.11.0)

eng/semconv/bootstrap-weaver.sh

[info] 40-40: Double quote to prevent globbing and word splitting.

(SC2086)

🔇 Additional comments (2)

eng/semconv/templates/registry/qyl/weaver.yaml (1)

14-77: Triple-maintained allowlist — already flagged.

The include_prefixes duplication concern was raised in a prior review cycle. Deferring to that comment.

eng/semconv/templates/registry/qyl/semconv.ts.j2 (1)

12-26: Enum values omission — already flagged.

Prior review noted the template drops *Values objects. Deferring to that comment.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Hardcoded semconv_version creates version drift risk.

Version 1.40.0 is duplicated here and in bootstrap-weaver.sh (SEMCONV_TAG). Upgrading requires synchronized edits. Consider extracting the version to a single source (e.g., a .env file or the manifest) that both scripts and templates consume.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@eng/semconv/templates/registry/qyl/weaver.yaml` at line 10, The hardcoded
semconv_version ("1.40.0") in weaver.yaml risks drift; centralize the version so
both semconv_version and the bootstrap script (which uses SEMCONV_TAG) read a
single source. Update semconv_version to reference a shared variable (e.g., load
from a .env or manifest) and change bootstrap-weaver.sh to read SEMCONV_TAG from
the same source (or export it from the manifest loader), ensuring the symbols
semconv_version and SEMCONV_TAG are derived from one canonical setting so
upgrades only require one edit.

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/qyl.contracts/Attributes/GenAiAttributes.cs (1)

203-216: ⚠️ Potential issue | 🟠 Major

Document governance criteria for qyl extensions.

Two sections marked // qyl extensions add custom operation and provider constants beyond OTel upstream. No documentation explains:

• Criteria for adding qyl-specific values
• Naming conventions for extensions
• How to track which values are qyl-owned vs upstream
• Whether extensions should move to a separate QylGenAiExtensions class to prevent confusion

When OTel adds new providers or operations, maintainers must manually distinguish qyl extensions from missed upstream additions.

Also applies to: 267-301

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/qyl.contracts/Attributes/GenAiAttributes.cs` around lines 203 - 216, Add
clear governance docs and reorganize qyl-specific constants to avoid confusion
with upstream OTel values: document criteria for adding qyl extensions, naming
conventions, and tracking policy (e.g., prefixing or registry) within the
GenAiAttributes class comments and move all qyl-only constants (ImageGeneration,
AudioTranscription, TextToSpeech, Rerank and the other qyl entries at 267-301)
into a separate QylGenAiExtensions static class; update XML summaries for both
GenAiAttributes and QylGenAiExtensions to state which values are qyl-owned vs
upstream and include a short maintenance note describing how to reconcile with
OTel upstream additions.

src/qyl.collector/Storage/promoted-columns.g.sql (1)

1-1369: ⚠️ Potential issue | 🟠 Major

Collector ingest still writes the old promoted-column contract.

This regeneration widens the schema, but src/qyl.collector/Storage/DuckDbStore.cs:42-66 still inserts the fixed 26-column span payload and src/qyl.collector/Storage/SpanRowMapper.cs:1-60 only understands that subset. The new promoted columns will stay NULL, so the schema now advertises fast-query fields the collector never populates. Keep the template allowlist aligned with the ingest contract, or update the ingest generator in the same PR.

As per coding guidelines "**/*.g.sql: Never hand-edit *.g.sql files. Fix the generator input instead."

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@eng/semconv/bootstrap-weaver.sh`:
- Around line 25-39: The script reuses caches by existence only (WEAVER_DIR and
UPSTREAM_DIR) which can lead to stale artifacts when WEAVER_VERSION/WEAVER_ARCH
or SEMCONV_TAG change; update the logic to validate cache contents before reuse:
for the weaver artifact (weaver-${WEAVER_ARCH}/weaver) run a version check (or
read a metadata file written at download time) and compare the reported
version/arch to WEAVER_VERSION/WEAVER_ARCH, and if they differ remove the cached
WEAVER_DIR and re-download; for the semconv upstream (UPSTREAM_DIR) verify the
checked-out commit/tag (git -C "${UPSTREAM_DIR}" rev-parse --verify
"refs/tags/${SEMCONV_TAG}" or compare current HEAD to SEMCONV_TAG) and if it
does not match remove UPSTREAM_DIR and reclone; ensure these checks reference
WEAVER_DIR, WEAVER_VERSION, WEAVER_ARCH, UPSTREAM_DIR, and SEMCONV_TAG so the
cache is invalidated when pins change.

In `@eng/semconv/run-weaver.sh`:
- Around line 19-29: run-weaver.sh duplicates the platform-to-WEAVER_ARCH
mapping (the case that sets WEAVER_ARCH) already present in bootstrap-weaver.sh,
causing drift; extract the detection logic into a single sourced helper (e.g., a
script that sets WEAVER_ARCH or a function like detect_weaver_arch) and update
both run-weaver.sh and bootstrap-weaver.sh to source/call that helper and use
the resulting WEAVER_ARCH variable (or exported value) so they both resolve the
same .tools/weaver/weaver-${WEAVER_ARCH} path consistently.

In `@src/qyl.contracts/Attributes/DbAttributes.cs`:
- Around line 1-6: Add a CI validation that compares the string constants in the
hand-maintained semconv facade classes (DbAttributes, GenAiAttributes,
McpAttributes) against the authoritative OTel semconv registry (either the
Weaver-generated output or the upstream semconv JSON) and fail the build on any
mismatch; implement this by adding a unit/integration test (e.g.,
SemconvValidationTests) that loads the upstream registry artifact at runtime,
enumerates the expected keys, reflects over the constants in
DbAttributes/GenAiAttributes/McpAttributes to collect their values, and asserts
set equality (reporting missing/extra keys), then wire that test into CI as a
required step so any drift breaks the pipeline.

In `@src/qyl.contracts/Attributes/GenAiAttributes.cs`:
- Around line 1-6: Add a CI validation step that parses the constant values from
the hand-maintained attribute classes (e.g., GenAiAttributes.cs,
McpAttributes.cs, DbAttributes.cs) and compares them against the
Weaver-validated outputs (or the upstream OTel registry); implement this as a
new validation routine (e.g., ValidateCsSemconvAlignment or
CompareCsSemconvWithWeaver) invoked from the CI pipeline (or via
BuildPipeline.cs hook) that loads the Weaver-generated semconv reference (or
registry export), extracts key names/values, loads the corresponding const
string fields from the classes (GenAiAttributes, McpAttributes, DbAttributes),
and fails the job with a clear mismatch report when any constant differs so
drift is detected before merge.

In `@src/qyl.contracts/Attributes/McpAttributes.cs`:
- Around line 1-6: The three hand-maintained semconv facades (McpAttributes.cs,
DbAttributes.cs, GenAiAttributes.cs) lack automated drift detection; add a small
verification step (e.g., a script named verify-semconv or verify_semconv.sh)
that fetches the upstream OTel 1.40.0 semconv registry (JSON), parses the
expected keys, and compares them to the constants defined in
McpAttributes.cs/DbAttributes.cs/GenAiAttributes.cs, exiting non-zero on any
mismatch; wire this script into CI (a pipeline job) and optionally into a
pre-commit hook so mismatches fail the build. Alternatively, restore Weaver’s C#
template generation and replace the hand-maintained files with generated
outputs, adding that generation step to CI to keep the files in sync
automatically.

---

Outside diff comments:
In `@src/qyl.contracts/Attributes/GenAiAttributes.cs`:
- Around line 203-216: Add clear governance docs and reorganize qyl-specific
constants to avoid confusion with upstream OTel values: document criteria for
adding qyl extensions, naming conventions, and tracking policy (e.g., prefixing
or registry) within the GenAiAttributes class comments and move all qyl-only
constants (ImageGeneration, AudioTranscription, TextToSpeech, Rerank and the
other qyl entries at 267-301) into a separate QylGenAiExtensions static class;
update XML summaries for both GenAiAttributes and QylGenAiExtensions to state
which values are qyl-owned vs upstream and include a short maintenance note
describing how to reconcile with OTel upstream additions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: fccaef16-ed8c-4742-a393-8175f2a7ffa4

📥 Commits

Reviewing files that changed from the base of the PR and between 4a907d4 and 9ff826e.

⛔ Files ignored due to path filters (1)

• eng/semconv/package-lock.json is excluded by !**/package-lock.json, !**/package-lock.json and included by eng/**

📒 Files selected for processing (13)

• eng/build/BuildPipeline.cs
• eng/semconv/CHANGELOG.md
• eng/semconv/bootstrap-weaver.sh
• eng/semconv/generate-semconv.ts
• eng/semconv/package.json
• eng/semconv/qyl-extensions.json
• eng/semconv/run-weaver.sh
• eng/semconv/tsconfig.json
• src/qyl.collector/Storage/promoted-columns.g.sql
• src/qyl.contracts/Attributes/DbAttributes.cs
• src/qyl.contracts/Attributes/GenAiAttributes.cs
• src/qyl.contracts/Attributes/McpAttributes.cs
• src/qyl.dashboard/src/lib/semconv.ts

💤 Files with no reviewable changes (5)

• eng/semconv/tsconfig.json
• eng/semconv/package.json
• eng/semconv/CHANGELOG.md
• eng/semconv/qyl-extensions.json
• eng/semconv/generate-semconv.ts

📜 Review details

⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Analyze (csharp)

🧰 Additional context used

📓 Path-based instructions (5)

**/*.cs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.cs: New .cs files must be UTF-8 with BOM. Required for dotnet format to behave. .editorconfig declares charset = utf-8-bom under [*.cs].
Include copyright header // Copyright (c) 2025-2026 ancplua at the top of .cs files (personal repo; MAF uses the Microsoft header).
XML documentation is required on all public methods and classes. GenerateDocumentationFile=true is set in Directory.Build.props.
Use the Async suffix for any method returning Task / ValueTask — including test methods.
Declare private classes as sealed unless intentionally subclassed.
Use C# 14 with preview features enabled. File-scoped namespaces, primary constructors, required init properties, pattern matching, switch expressions over if-else.
No suppression of warnings. Forbidden: #pragma warning disable, [SuppressMessage], <NoWarn>. Exception: upstream sample repos demonstrating experimental APIs. qyl's WarningsAsErrors=CA1816;CA2012;CA2016 is already minimal — add more rules, never subtract.
Use IIncrementalGenerator only for generators, with ForAttributeWithMetadataName, value-equatable models, raw strings over SyntaxFactory. Never store ISymbol in models. Test generators via ANcpLua.Roslyn.Utilities test infrastructure.
Never use runtime reflection as a control mechanism, dynamic / ExpandoObject, blocking async (.Result / .Wait()), or any analyzer besides ANcpLua.Analyzers. Do not suppress null ! when the code can be rewritten.
Arrange / Act / Assert comments in test methods. Use the project's FakeChatClient (tests/qyl.collector.tests/Instrumentation/) for IChatClient doubles — do NOT hand-roll Moq<IChatClient>.
Wrap every IChatClient AND the AIAgent with the telemetry pipeline — both layers, not one. Use UseQylTelemetry (the qyl wrapper) on the chat client, not a hand-rolled UseFunctionInvocation().UseOpenTelemetry(...) chain.
When constructing IChatClient, set EnableSensitiveData = null to defer to `OTEL_INSTRUMENTATION_GE...

Files:

• src/qyl.contracts/Attributes/DbAttributes.cs
• src/qyl.contracts/Attributes/GenAiAttributes.cs
• eng/build/BuildPipeline.cs
• src/qyl.contracts/Attributes/McpAttributes.cs

src/**/*.cs

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.cs: Do not let MAF types leak into qyl.contracts. Do not let the dashboard call runtime code directly — it goes through the collector REST API. Dependency direction is one-way: contracts at bottom, generators separate, runtime depends on contracts + outputs.
Reference ~/Apex.AgenticEntityExtractor/ (canonical qyl consumer shape) when building new qyl services. Mirror the three-builder-interface + fluent-middleware pattern from: Agents/ExtractorAgentsBuilder.cs, Clients/ExtractorChatClientBuilder.cs, Workflows/ExtractorWorkflowBuilder.cs, Program.cs.

Files:

• src/qyl.contracts/Attributes/DbAttributes.cs
• src/qyl.contracts/Attributes/GenAiAttributes.cs
• src/qyl.contracts/Attributes/McpAttributes.cs

---

⚙️ CodeRabbit configuration file

src/**/*.cs: C#/.NET codebase. Review for: idiomatic C# patterns, proper async/await (no sync-over-async, no fire-and-forget without justification), correct IDisposable/IAsyncDisposable, null safety, and adherence to existing patterns. Flag new public API surface. Check DI lifetime correctness (scoped vs singleton vs transient).
ARCHITECTURAL INVARIANTS — flag violations as blocking: - Every new injectable service must register OpenTelemetry instrumentation (ActivitySource or Meter). - Every new DuckDB write path must handle backpressure (bounded channel or semaphore). - No hardcoded connection strings, paths, or magic strings — use IOptions or IConfiguration. - No new dependencies on Sentry-specific types in core/ — Sentry is an optional backend, not the identity. - CancellationToken must be threaded through all async public methods.

Files:

• src/qyl.contracts/Attributes/DbAttributes.cs
• src/qyl.contracts/Attributes/GenAiAttributes.cs
• src/qyl.contracts/Attributes/McpAttributes.cs

eng/**

⚙️ CodeRabbit configuration file

Build and deployment infrastructure. Review for: correct MSBuild property usage, Nuke build target dependencies, Docker multi-stage build efficiency, and CI/CD pipeline correctness. Flag hardcoded paths, secrets, or platform-specific assumptions.

Files:

• eng/build/BuildPipeline.cs
• eng/semconv/bootstrap-weaver.sh
• eng/semconv/run-weaver.sh

src/qyl.dashboard/**

⚙️ CodeRabbit configuration file

React/TypeScript dashboard. Review for: single-responsibility components, proper hook usage (no hooks in conditionals, correct dependency arrays), TypeScript strictness (no any unless justified with a comment), and basic accessibility. Uses Tailwind — flag inline styles or custom CSS that duplicates utility classes. Flag any direct fetch() calls that bypass the shared API client.

Files:

• src/qyl.dashboard/src/lib/semconv.ts

**/*.g.sql

📄 CodeRabbit inference engine (AGENTS.md)

Never hand-edit *.g.sql files. Fix the generator input instead.

Files:

• src/qyl.collector/Storage/promoted-columns.g.sql

🧠 Learnings (10)

📓 Common learnings

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: src/qyl.mcp/CLAUDE.md:0-0
Timestamp: 2026-04-21T00:49:29.212Z
Learning: Applies to src/qyl.mcp/src/qyl.mcp.generators/**/*.cs : `src/qyl.mcp.generators/` must emit `QylToolManifest` with `ToolTypes[]`, `ToolDescriptors[]`, and `CreateTools()`

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to src/qyl.mcp/** : When MAF conventions conflict with qyl conventions, MAF wins. qyl consumes MAF and must stay aligned with upstream .NET patterns. Read the `microsoft-agent-framework` global skill plus the `microsoft-agent-framework-qyl` overlay in `.claude/skills/`.

Applied to files:

• src/qyl.contracts/Attributes/DbAttributes.cs
• src/qyl.contracts/Attributes/GenAiAttributes.cs
• src/qyl.contracts/Attributes/McpAttributes.cs

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: qyl emits OTel GenAI semconv 1.40 with required attributes: `gen_ai.system`, `gen_ai.request.model`, `gen_ai.response.model`, `gen_ai.usage.input_tokens`, `gen_ai.usage.output_tokens`, `gen_ai.operation.name`, `gen_ai.tool.call.id`, `gen_ai.tool.name`, `gen_ai.agent.name`, `gen_ai.agent.id`.

Applied to files:

• src/qyl.contracts/Attributes/DbAttributes.cs
• src/qyl.contracts/Attributes/GenAiAttributes.cs
• src/qyl.contracts/Attributes/McpAttributes.cs
• src/qyl.dashboard/src/lib/semconv.ts
• src/qyl.collector/Storage/promoted-columns.g.sql

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to src/**/Program.cs : Every qyl service must wire OTel at the composition root: `ResourceBuilder.CreateDefault().AddService(...).AddAttributes(...)` plus `.UseQylTelemetry()` on IChatClient and `.UseOpenTelemetry()` on AIAgent.

Applied to files:

• src/qyl.contracts/Attributes/DbAttributes.cs
• src/qyl.contracts/Attributes/GenAiAttributes.cs
• src/qyl.contracts/Attributes/McpAttributes.cs

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to **/*.cs : Do NOT hand-add tools to DI, MCP registration, or skill catalogs. Use `[QylSkill(QylSkillKind.X)]` and `[QylCapability]` attributes instead — the generator handles registration from the attribute.

Applied to files:

• src/qyl.contracts/Attributes/GenAiAttributes.cs
• src/qyl.contracts/Attributes/McpAttributes.cs

📚 Learning: 2026-04-21T00:49:29.212Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: src/qyl.mcp/CLAUDE.md:0-0
Timestamp: 2026-04-21T00:49:29.212Z
Learning: Applies to src/qyl.mcp/src/qyl.mcp.generators/**/*.cs : `src/qyl.mcp.generators/` must emit `QylToolManifest` with `ToolTypes[]`, `ToolDescriptors[]`, and `CreateTools()`

Applied to files:

• eng/build/BuildPipeline.cs
• src/qyl.contracts/Attributes/McpAttributes.cs

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to **/*.cs : Compile-time wiring over runtime reflection: the generator owns DI registration, MCP tool registration, and capability catalogs. Use `[QylSkill]` + `[QylCapability]` attributes instead of hand-registering tools.

Applied to files:

• src/qyl.contracts/Attributes/McpAttributes.cs

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to src/qyl.mcp/**/*.cs : Declare tool class with `[McpServerToolType]` and `[QylSkill(QylSkillKind.X)]`. Declare tool methods with `[QylCapability("id", Starting|FollowUp)]`. Never hand-register tools — let the generator produce `RegisterTools()`, `RegisterServices()`, `Capabilities[]`, `ToolDescriptors[]`.

Applied to files:

• src/qyl.contracts/Attributes/McpAttributes.cs

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to **/*.cs : Include copyright header `// Copyright (c) 2025-2026 ancplua` at the top of .cs files (personal repo; MAF uses the Microsoft header).

Applied to files:

• src/qyl.contracts/Attributes/McpAttributes.cs

📚 Learning: 2026-04-21T00:49:29.212Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: src/qyl.mcp/CLAUDE.md:0-0
Timestamp: 2026-04-21T00:49:29.212Z
Learning: Applies to src/qyl.mcp/**/*.cs : Use `System.Text.Json` instead of Newtonsoft for JSON serialization in C# code

Applied to files:

• src/qyl.contracts/Attributes/McpAttributes.cs

🪛 Shellcheck (0.11.0)

eng/semconv/bootstrap-weaver.sh

[info] 42-42: Double quote to prevent globbing and word splitting.

(SC2086)

🔇 Additional comments (1)

src/qyl.dashboard/src/lib/semconv.ts (1)

1-1368: No orphaned semconv exports detected after Weaver cutover.

Verification confirms that while the generator did remove enum-like *Values exports, all callers in the codebase have been properly updated to use individual string constants (e.g., GEN_AI_TOOL_NAME, GEN_AI_TOOL_DESCRIPTION). No build-time failures will occur from stranded imports.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

The version pins are comments, not cache invariants.

Both caches are reused on existence alone. If a developer already has .tools/weaver or .tools/semconv-upstream from an older WEAVER_VERSION or SEMCONV_TAG, this script silently generates against stale inputs even though the pins changed. Validate the cached binary/tag before reuse, or blow the cache away when the pins do not match.

Suggested hardening

 mkdir -p "${WEAVER_DIR}"
-if [ ! -x "${WEAVER_DIR}/weaver-${WEAVER_ARCH}/weaver" ]; then
+WEAVER_BIN="${WEAVER_DIR}/weaver-${WEAVER_ARCH}/weaver"
+if [ ! -x "${WEAVER_BIN}" ] || ! "${WEAVER_BIN}" --version 2>/dev/null | grep -Fq "${WEAVER_VERSION#v}"; then
+  rm -rf "${WEAVER_DIR}/weaver-${WEAVER_ARCH}"
   echo "Downloading Weaver ${WEAVER_VERSION} (${WEAVER_ARCH})..."
   curl -sL \
     "https://github.com/open-telemetry/weaver/releases/download/${WEAVER_VERSION}/weaver-${WEAVER_ARCH}.tar.xz" \
     -o "${WEAVER_DIR}/weaver.tar.xz"
   tar -xf "${WEAVER_DIR}/weaver.tar.xz" -C "${WEAVER_DIR}"
   rm "${WEAVER_DIR}/weaver.tar.xz"
 fi
 
-if [ ! -d "${UPSTREAM_DIR}" ]; then
+if [ ! -d "${UPSTREAM_DIR}/.git" ] || [ "$(git -C "${UPSTREAM_DIR}" describe --tags --exact-match 2>/dev/null || true)" != "${SEMCONV_TAG}" ]; then
+  rm -rf "${UPSTREAM_DIR}"
   echo "Cloning open-telemetry/semantic-conventions@${SEMCONV_TAG}..."
   git clone --depth 1 --branch "${SEMCONV_TAG}" \
     https://github.com/open-telemetry/semantic-conventions.git "${UPSTREAM_DIR}"
 fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@eng/semconv/bootstrap-weaver.sh` around lines 25 - 39, The script reuses
caches by existence only (WEAVER_DIR and UPSTREAM_DIR) which can lead to stale
artifacts when WEAVER_VERSION/WEAVER_ARCH or SEMCONV_TAG change; update the
logic to validate cache contents before reuse: for the weaver artifact
(weaver-${WEAVER_ARCH}/weaver) run a version check (or read a metadata file
written at download time) and compare the reported version/arch to
WEAVER_VERSION/WEAVER_ARCH, and if they differ remove the cached WEAVER_DIR and
re-download; for the semconv upstream (UPSTREAM_DIR) verify the checked-out
commit/tag (git -C "${UPSTREAM_DIR}" rev-parse --verify
"refs/tags/${SEMCONV_TAG}" or compare current HEAD to SEMCONV_TAG) and if it
does not match remove UPSTREAM_DIR and reclone; ensure these checks reference
WEAVER_DIR, WEAVER_VERSION, WEAVER_ARCH, UPSTREAM_DIR, and SEMCONV_TAG so the
cache is invalidated when pins change.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Resolve WEAVER_ARCH in one shared helper.

run-weaver.sh duplicates the platform matrix from bootstrap-weaver.sh. Those scripts must stay bit-for-bit aligned because they address the same .tools/weaver/weaver-${WEAVER_ARCH} cache, and this mapping already regressed once. Extract the detection into a sourced helper so bootstrap and generate cannot drift again.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@eng/semconv/run-weaver.sh` around lines 19 - 29, run-weaver.sh duplicates the
platform-to-WEAVER_ARCH mapping (the case that sets WEAVER_ARCH) already present
in bootstrap-weaver.sh, causing drift; extract the detection logic into a single
sourced helper (e.g., a script that sets WEAVER_ARCH or a function like
detect_weaver_arch) and update both run-weaver.sh and bootstrap-weaver.sh to
source/call that helper and use the resulting WEAVER_ARCH variable (or exported
value) so they both resolve the same .tools/weaver/weaver-${WEAVER_ARCH} path
consistently.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check if verification tooling exists for semconv alignment

# Look for validation scripts that check semconv constants against upstream
fd -e sh -e ps1 -e ts -e js . eng/ | xargs rg -l 'semconv.*valid|verify.*semconv|check.*1\.40'

# Check if Weaver templates include validation output
fd weaver.yaml eng/

# Check for any drift-detection automation in build targets
rg -n 'semconv.*verify|validate.*semconv' --glob '*.targets' --glob '*.props' --glob 'build.cs'

Repository: Alexander-Nachtmann/qyl

Length of output: 113

---

🏁 Script executed:

# First, examine the weaver.yaml file to understand the current setup
cat -n eng/semconv/templates/registry/qyl/weaver.yaml

Repository: Alexander-Nachtmann/qyl

Length of output: 2405

---

🏁 Script executed:

# Check the DbAttributes.cs file to see the actual constants being maintained
cat -n src/qyl.contracts/Attributes/DbAttributes.cs | head -130

Repository: Alexander-Nachtmann/qyl

Length of output: 5160

---

🏁 Script executed:

# Look for any CI validation or build-time checks for semconv alignment
rg -n "semconv|DbAttributes" --glob "*.yml" --glob "*.yaml" .github/ || echo "No CI found for semconv"

Repository: Alexander-Nachtmann/qyl

Length of output: 90

---

🏁 Script executed:

# Check if there's a build script or generator that validates the constants
find . -name "*.cs" -type f | xargs rg -l "semconv.*validate|validate.*constant" | head -10

Repository: Alexander-Nachtmann/qyl

Length of output: 49

---

🏁 Script executed:

# Look for any references to OTel 1.40.0 or semconv version pinning
rg "1\.40\.0|semconv.*version" --glob "*.cs" --glob "*.json" --glob "*.yaml" --glob "*.props"

Repository: Alexander-Nachtmann/qyl

Length of output: 5399

---

Add CI validation for semconv attribute constants.

Hand-maintaining semconv facades without automated drift detection creates long-term risk. The comment promises to "bump semconv keys by hand when upstream moves," but no tooling exists to detect upstream semconv versions or validate that current constants (e.g., db.system.name, gen_ai.model.name) remain aligned with OTel 1.40.0.

This pattern affects three files: DbAttributes.cs, GenAiAttributes.cs, McpAttributes.cs. While OTel semconv Stable attributes rarely break, a CI check that validates these string constants against the upstream semconv registry (either via Weaver output or a direct registry comparison) would eliminate drift risk and align with the codebase's "compile-time wiring" philosophy.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/qyl.contracts/Attributes/DbAttributes.cs` around lines 1 - 6, Add a CI
validation that compares the string constants in the hand-maintained semconv
facade classes (DbAttributes, GenAiAttributes, McpAttributes) against the
authoritative OTel semconv registry (either the Weaver-generated output or the
upstream semconv JSON) and fail the build on any mismatch; implement this by
adding a unit/integration test (e.g., SemconvValidationTests) that loads the
upstream registry artifact at runtime, enumerates the expected keys, reflects
over the constants in DbAttributes/GenAiAttributes/McpAttributes to collect
their values, and asserts set equality (reporting missing/extra keys), then wire
that test into CI as a required step so any drift breaks the pipeline.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check if verification tooling exists for semconv alignment

# Look for validation scripts that check semconv constants against upstream
fd -e sh -e ps1 -e ts -e js . eng/ | xargs rg -l 'semconv.*valid|verify.*semconv|check.*1\.40'

# Check if Weaver templates include validation output
fd weaver.yaml eng/

# Check for any drift-detection automation in build targets
rg -n 'semconv.*verify|validate.*semconv' --glob '*.targets' --glob '*.props' --glob 'build.cs'

Repository: Alexander-Nachtmann/qyl

Length of output: 113

---

🏁 Script executed:

cat -n eng/semconv/templates/registry/qyl/weaver.yaml

Repository: Alexander-Nachtmann/qyl

Length of output: 2405

---

🏁 Script executed:

head -50 src/qyl.contracts/Attributes/GenAiAttributes.cs

Repository: Alexander-Nachtmann/qyl

Length of output: 2040

---

🏁 Script executed:

rg -n 'semconv|Weaver|1\.40' src/qyl.contracts/Attributes/GenAiAttributes.cs -A 3

Repository: Alexander-Nachtmann/qyl

Length of output: 835

---

🏁 Script executed:

# Check if there are any build or tooling scripts that interact with semconv
fd -type f \( -name '*.yaml' -o -name '*.yml' -o -name '*.json' \) eng/semconv/ | head -20

Repository: Alexander-Nachtmann/qyl

Length of output: 237

---

🏁 Script executed:

# Look for documentation or comments about the maintenance process
rg -n 'hand.edit|hand.maintain|Weaver migration' src/qyl.contracts/ -B 2 -A 2

Repository: Alexander-Nachtmann/qyl

Length of output: 2271

---

🏁 Script executed:

# Check if the weaver-generated outputs exist
find . -name "semconv.ts" -o -name "promoted-columns.g.sql" -o -name "semconv.g.tsp" 2>/dev/null

Repository: Alexander-Nachtmann/qyl

Length of output: 193

---

🏁 Script executed:

# Look for qyl-extensions.json
find . -name "qyl-extensions.json" 2>/dev/null

Repository: Alexander-Nachtmann/qyl

Length of output: 49

---

🏁 Script executed:

# Check if there's a build step invoking weaver for .cs generation
rg -n "weaver|Weaver" --glob "*.targets" --glob "*.props" --glob "*.cs" --glob "*.yml" --glob "*.yaml"

Repository: Alexander-Nachtmann/qyl

Length of output: 1291

---

🏁 Script executed:

# Verify whether the .cs attribute files are actually generated or hand-written
# by checking if they appear in any generation/build configuration
rg -n "GenAiAttributes|McpAttributes|DbAttributes" --glob "*.targets" --glob "*.props"

Repository: Alexander-Nachtmann/qyl

Length of output: 49

---

🏁 Script executed:

# Check the eng/semconv directory structure to understand the full pipeline
ls -la eng/semconv/

Repository: Alexander-Nachtmann/qyl

Length of output: 430

---

🏁 Script executed:

# Check for any validation or sync documentation
rg -n "sync|validate|drift" eng/semconv/ src/qyl.contracts/

Repository: Alexander-Nachtmann/qyl

Length of output: 562

---

🏁 Script executed:

# Check if there's a manual process documented for updating .cs files
rg -n "semconv|GenAiAttributes|McpAttributes" eng/ -A 2 -B 2 --glob "*.md" --glob "*.txt"

Repository: Alexander-Nachtmann/qyl

Length of output: 49

---

🏁 Script executed:

# Examine the manifest to understand the overall pipeline
cat eng/semconv/registry-qyl/manifest.yaml

Repository: Alexander-Nachtmann/qyl

Length of output: 1273

---

🏁 Script executed:

# Check BuildPipeline.cs around the weaver invocation to see if .cs generation is part of pipeline
sed -n '165,200p' eng/build/BuildPipeline.cs

Repository: Alexander-Nachtmann/qyl

Length of output: 1923

---

Add validation to detect .cs semconv drift from weaver-generated outputs.

The GenAiAttributes.cs file is intentionally decoupled from the Weaver pipeline — BuildPipeline.cs explicitly logs "Facades under src/qyl.contracts/Attributes/*.cs are hand-maintained" while Weaver validates TypeScript, SQL, and TypeSpec outputs against OTel 1.40.0. This creates undetected drift risk: when OpenTelemetry publishes a new semconv version, Weaver will catch it for .ts and .sql files, but the .cs constant values in GenAiAttributes, McpAttributes, and DbAttributes will silently diverge.

Add a CI check that compares .cs constant values against the Weaver-validated outputs (or the upstream registry directly) to catch misalignment before merge.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/qyl.contracts/Attributes/GenAiAttributes.cs` around lines 1 - 6, Add a CI
validation step that parses the constant values from the hand-maintained
attribute classes (e.g., GenAiAttributes.cs, McpAttributes.cs, DbAttributes.cs)
and compares them against the Weaver-validated outputs (or the upstream OTel
registry); implement this as a new validation routine (e.g.,
ValidateCsSemconvAlignment or CompareCsSemconvWithWeaver) invoked from the CI
pipeline (or via BuildPipeline.cs hook) that loads the Weaver-generated semconv
reference (or registry export), extracts key names/values, loads the
corresponding const string fields from the classes (GenAiAttributes,
McpAttributes, DbAttributes), and fails the job with a clear mismatch report
when any constant differs so drift is detected before merge.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check if verification tooling exists for semconv alignment

# Look for validation scripts that check semconv constants against upstream
fd -e sh -e ps1 -e ts -e js . eng/ | xargs rg -l 'semconv.*valid|verify.*semconv|check.*1\.40'

# Check if Weaver templates include validation output
fd weaver.yaml eng/

# Check for any drift-detection automation in build targets
rg -n 'semconv.*verify|validate.*semconv' --glob '*.targets' --glob '*.props' --glob 'build.cs'

Repository: Alexander-Nachtmann/qyl

Length of output: 113

---

🏁 Script executed:

# Read the weaver.yaml file to understand Weaver configuration
cat eng/semconv/templates/registry/qyl/weaver.yaml

Repository: Alexander-Nachtmann/qyl

Length of output: 1754

---

🏁 Script executed:

# Read the McpAttributes.cs file to see full scope and assess hand-maintenance
cat -n src/qyl.contracts/Attributes/McpAttributes.cs

Repository: Alexander-Nachtmann/qyl

Length of output: 4020

---

🏁 Script executed:

# Search for related semconv facade files mentioned in comment ("all three files")
fd -e cs . src/qyl.contracts/Attributes/ | rg -i 'semconv|otel|attributes'

Repository: Alexander-Nachtmann/qyl

Length of output: 205

---

🏁 Script executed:

# Check git log to see when the migration happened and any related commits
git log --oneline -20 -- src/qyl.contracts/Attributes/McpAttributes.cs eng/semconv/

Repository: Alexander-Nachtmann/qyl

Length of output: 147

---

🏁 Script executed:

# Check if Weaver is still configured to run in build pipeline
rg -l 'weaver' --glob '*.targets' --glob '*.props' --glob 'Directory.Build.*' .

Repository: Alexander-Nachtmann/qyl

Length of output: 49

---

🏁 Script executed:

# Search for generated C# files (.g.cs) related to semconv
fd -e 'g.cs' . src/qyl.contracts/Attributes/

Repository: Alexander-Nachtmann/qyl

Length of output: 49

---

🏁 Script executed:

# Check Weaver templates directory for C# generation templates
fd . eng/semconv/templates/ | grep -E '\.(j2|jinja|template)$'

Repository: Alexander-Nachtmann/qyl

Length of output: 228

---

🏁 Script executed:

# Check the headers of the other two semconv facade files
head -7 src/qyl.contracts/Attributes/DbAttributes.cs
head -7 src/qyl.contracts/Attributes/GenAiAttributes.cs

Repository: Alexander-Nachtmann/qyl

Length of output: 716

---

🏁 Script executed:

# Check full git log (not just recent) to understand the migration timeline
git log --oneline --all -- src/qyl.contracts/Attributes/McpAttributes.cs | head -10

Repository: Alexander-Nachtmann/qyl

Length of output: 147

---

🏁 Script executed:

# Check if weaver actually executes as part of build
rg -i 'weaver|semconv' Directory.Build.props Directory.Build.targets qyl.contracts.csproj 2>/dev/null || echo "Not found in build files"

Repository: Alexander-Nachtmann/qyl

Length of output: 91

---

Hand-maintained semconv facades lack drift detection across all three files.

Migration from Weaver-generated to hand-maintained C# constants (McpAttributes.cs, DbAttributes.cs, GenAiAttributes.cs) introduces spec drift risk without mitigation. Weaver generates TypeScript/SQL/TypeSpec outputs but explicitly does NOT generate the C# facades — they are intentionally hand-maintained. The header comment promises manual updates ("Bump semconv keys by hand when upstream moves") but provides no verification mechanism, validation automation, or integration into the build pipeline to detect misalignment with OTel 1.40.0.

Add either: (1) a verification script that validates the C# constants against the upstream OTel semconv registry, integrated into CI/pre-commit hooks, or (2) restore Weaver C# template generation to eliminate manual maintenance.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/qyl.contracts/Attributes/McpAttributes.cs` around lines 1 - 6, The three
hand-maintained semconv facades (McpAttributes.cs, DbAttributes.cs,
GenAiAttributes.cs) lack automated drift detection; add a small verification
step (e.g., a script named verify-semconv or verify_semconv.sh) that fetches the
upstream OTel 1.40.0 semconv registry (JSON), parses the expected keys, and
compares them to the constants defined in
McpAttributes.cs/DbAttributes.cs/GenAiAttributes.cs, exiting non-zero on any
mismatch; wire this script into CI (a pipeline job) and optionally into a
pre-commit hook so mismatches fail the build. Alternatively, restore Weaver’s C#
template generation and replace the hand-maintained files with generated
outputs, adding that generation step to CI to keep the files in sync
automatically.

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@eng/semconv/templates/registry/qyl/semconv.g.tsp.j2`:
- Around line 73-88: The child/camel derivation logic is duplicated (the
computations that set child and camel in the templated Keys alias block and
again in the model fields), so extract that logic into a reusable Jinja macro
(e.g., a macro compute_child_and_camel(name) placed near the top of
eng/semconv/templates/registry/qyl/semconv.g.tsp.j2) that returns or sets the
pascal/camel forms; then replace the inline sets (the lines that define child
and camel used for Keys aliases and the similar block used for model fields)
with calls to this macro (referencing the existing symbols child and camel where
used by alias generation and model field generation) to ensure a single source
of truth for the conversion logic.
- Around line 138-150: The template's type-mapping conditional (uses attr.type
and sets tsp_type) currently converts primitives and arrays but silently treats
semconv template[...] types as generic "string"; update the conditional in
eng/semconv/templates/registry/qyl/semconv.g.tsp.j2 to detect values matching
the pattern "template[...]" (e.g., using a startswith or regex check on
attr.type) and map them to an appropriate tsp_type (for example preserve
"template[string]" → "template[string]" or map to a distinct type like
"template[string]"/"string_map" per project convention), or if you intend to
keep the fallback, explicitly emit a comment in the template noting that
template[...] types intentionally collapse to "string" so reviewers/readers see
the behavior; ensure references to attr.type and tsp_type in the conditional are
updated accordingly.
- Around line 13-16: The reserved keyword list in the template's reserved
variable is missing many keywords introduced by TypeSpec (so identifiers like
statemachine, macro, package, metadata, env, arg, declare, array, struct,
record, module, trait, this, self, super, keyof, with, implements, impl,
satisfies, flag, auto, partial, private, public, protected, internal, sealed,
local, async can be emitted unescaped); update the set assigned to reserved in
the semconv.g.tsp.j2 template to include these additional TypeSpec reserved
words so the safe(ident) macro will correctly backtick-escape them when used.
Ensure you modify the reserved list near the top of the file (the reserved
variable declaration) and run tests/linting to confirm generated TypeSpec is
valid.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1bfe5d54-52b4-46a4-884a-e3df4dca213d

📥 Commits

Reviewing files that changed from the base of the PR and between 9ff826e and e0b44f3.

⛔ Files ignored due to path filters (1)

• core/specs/generated/semconv.g.tsp is excluded by !**/generated/** and included by core/**

📒 Files selected for processing (3)

• eng/semconv/run-weaver.sh
• eng/semconv/templates/registry/qyl/semconv.g.tsp.j2
• eng/semconv/templates/registry/qyl/weaver.yaml

📜 Review details

🧰 Additional context used

📓 Path-based instructions (1)

eng/**

⚙️ CodeRabbit configuration file

Build and deployment infrastructure. Review for: correct MSBuild property usage, Nuke build target dependencies, Docker multi-stage build efficiency, and CI/CD pipeline correctness. Flag hardcoded paths, secrets, or platform-specific assumptions.

Files:

• eng/semconv/templates/registry/qyl/weaver.yaml
• eng/semconv/run-weaver.sh
• eng/semconv/templates/registry/qyl/semconv.g.tsp.j2

🧠 Learnings (5)

📓 Common learnings

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: src/qyl.mcp/CLAUDE.md:0-0
Timestamp: 2026-04-21T00:49:29.212Z
Learning: Applies to src/qyl.mcp/src/qyl.mcp.generators/**/*.cs : `src/qyl.mcp.generators/` must emit `QylToolManifest` with `ToolTypes[]`, `ToolDescriptors[]`, and `CreateTools()`

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to **/*.cs : Use standard OTel environment variables (OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_PROTOCOL, OTEL_SERVICE_NAME, ENABLE_INSTRUMENTATION) instead of qyl-invented ones. Prefer these over hardcoded config.

Applied to files:

• eng/semconv/templates/registry/qyl/weaver.yaml

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: qyl emits OTel GenAI semconv 1.40 with required attributes: `gen_ai.system`, `gen_ai.request.model`, `gen_ai.response.model`, `gen_ai.usage.input_tokens`, `gen_ai.usage.output_tokens`, `gen_ai.operation.name`, `gen_ai.tool.call.id`, `gen_ai.tool.name`, `gen_ai.agent.name`, `gen_ai.agent.id`.

Applied to files:

• eng/semconv/templates/registry/qyl/semconv.g.tsp.j2

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to **/*.g.cs : Never hand-edit *.g.cs files. Fix the generator input (TypeSpec model, attribute, routing table) instead.

Applied to files:

• eng/semconv/templates/registry/qyl/semconv.g.tsp.j2

📚 Learning: 2026-04-21T00:50:26.706Z

Learnt from: CR
Repo: Alexander-Nachtmann/qyl PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-21T00:50:26.706Z
Learning: Applies to core/openapi/openapi.yaml : Never hand-edit core/openapi/openapi.yaml. Fix the TypeSpec source instead.

Applied to files:

• eng/semconv/templates/registry/qyl/semconv.g.tsp.j2

🔇 Additional comments (7)

eng/semconv/templates/registry/qyl/weaver.yaml (1)

79-93: Template configuration is structurally sound.

The three template registrations correctly use semconv_grouped_attributes filter with application_mode: single, matching the iteration pattern in the Jinja templates. Output filenames align with the destinations in run-weaver.sh.

eng/semconv/run-weaver.sh (4)

28-35: Path definitions align with BuildPipeline.cs destinations.

The TS_DEST, SQL_DEST, and TSP_DEST variables match the paths documented in eng/build/BuildPipeline.cs:190-193. The staging directory under eng/semconv/out keeps generated artifacts gitignored as intended.

---

37-41: Prerequisite validation is adequate.

The -x check for WEAVER_BIN and -d check for UPSTREAM_REGISTRY fail fast with actionable guidance pointing to the bootstrap script.

---

43-53: Generation and installation sequence is correct.

The rm -rf clears stale staging artifacts before generation. The install -m 0644 ensures consistent permissions across platforms. The mkdir -p for TSP_DEST handles the case where core/specs/generated/ doesn't exist.

---

19-26: No action needed. Weaver v0.22.1 does not publish an aarch64-unknown-linux-gnu binary; the pre-built assets are limited to aarch64-apple-darwin, x86_64-apple-darwin, x86_64-unknown-linux-gnu, and Windows variants. The script correctly rejects ARM64 Linux with "Unsupported platform" and would fail at download if the proposed case were added. The platform matrix accurately reflects available binaries.

			> Likely an incorrect or invalid review comment.

eng/semconv/templates/registry/qyl/semconv.g.tsp.j2 (2)

30-34: Namespace and imports are correctly structured.

The OTel.SemConv namespace with TypeSpec.Http usage is appropriate for the HTTP decorator annotations and aligns with TypeSpec conventions.

---

41-48: The W3C Trace Context standard requires hex digits to be lowercase only (a-f, 0-9). The ABNF grammar defines hex using HEXDIGLC, and implementations must reject traceparent headers containing non-lowercase characters (e.g., uppercase A-F). The patterns in the code are correct and compliant with the specification. No changes needed.

			> Likely an incorrect or invalid review comment.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

TypeSpec language reserved keywords list

💡 Result:

TypeSpec does not have a published official list of reserved keywords in its documentation. The official docs state that "All keywords are reserved identifiers in TypeSpec" and can be used when escaped with backticks (e.g., model enum {}), but do not enumerate them. From the TypeSpec compiler source (microsoft/typespec repository), a recent merged pull request (#6416, March 2025) added/reserved these keywords for future use, causing breaking changes in @typespec/compiler: statemachine, macro, package, metadata, env, arg, declare, array, struct, record, module, trait, this, self, super, keyof, with, implements, impl, satisfies, flag, auto, partial, private, public, protected, internal, sealed, local, async Core structural keywords visible across docs include: model, namespace, interface, op, enum, alias, scalar, using, import, extends, is Reserved keywords cannot be used as unescaped identifiers in declaration names, type references, or base references in member expressions. Escape with backticks to use them as identifiers.

Citations:

• 1: https://typespec.io/docs/language-basics/identifiers/
• 2: Reserve keywords microsoft/typespec#6416
• 3: https://typespec.io/docs/language-basics/identifiers
• 4: https://typespec.io/docs/language-basics/overview/
• 5: https://typespec.io/docs/language-basics/overview
• 6: https://typespec.io/docs/language-basics/models/
• 7: https://typespec.io/docs/language-basics/namespaces/
• 8: https://typespec.io/docs/language-basics/values/

---

Reserved words list is incomplete and drifts from current TypeSpec spec.

TypeSpec PR #6416 (merged March 2025) reserved additional keywords including: statemachine, macro, package, metadata, env, arg, declare, array, struct, record, module, trait, this, self, super, keyof, with, implements, impl, satisfies, flag, auto, partial, private, public, protected, internal, sealed, local, async. The template's reserved set lacks these keywords. While the safe() macro correctly escapes identifiers, missing reserved keywords means they will emit unescaped, producing invalid TypeSpec if such attribute names appear in semconv definitions.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@eng/semconv/templates/registry/qyl/semconv.g.tsp.j2` around lines 13 - 16,
The reserved keyword list in the template's reserved variable is missing many
keywords introduced by TypeSpec (so identifiers like statemachine, macro,
package, metadata, env, arg, declare, array, struct, record, module, trait,
this, self, super, keyof, with, implements, impl, satisfies, flag, auto,
partial, private, public, protected, internal, sealed, local, async can be
emitted unescaped); update the set assigned to reserved in the semconv.g.tsp.j2
template to include these additional TypeSpec reserved words so the safe(ident)
macro will correctly backtick-escape them when used. Ensure you modify the
reserved list near the top of the file (the reserved variable declaration) and
run tests/linting to confirm generated TypeSpec is valid.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Duplicated camelCase conversion logic.

The child/camel derivation at lines 79-80 is repeated verbatim at lines 133-134. Extract a macro to avoid drift between Keys aliases and model fields.

Proposed refactor

+{%- macro attr_camel(attr) -%}
+{%- set child = attr.name.split('.')[1:] | join('.') | replace('.', '_') | pascal_case -%}
+{{ child[:1] | lower ~ child[1:] }}
+{%- endmacro -%}
+
 namespace Keys {
 {% for group in ctx | sort(attribute="root_namespace") %}
 {% if group.root_namespace in params.include_prefixes %}
   /** {{ group.root_namespace }}.* attribute keys */
   namespace {{ group.root_namespace | pascal_case }} {
 {% for attr in group.attributes | sort(attribute="name") %}
-{% set child = attr.name.split('.')[1:] | join('.') | replace('.', '_') | pascal_case %}
-{% set camel = child[:1] | lower ~ child[1:] %}
     /** "{{ attr.name }}" */
-    alias {{ safe(camel) }} = "{{ attr.name }}";
+    alias {{ safe(attr_camel(attr)) }} = "{{ attr.name }}";
 {% endfor %}

Also applies to: 132-156

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@eng/semconv/templates/registry/qyl/semconv.g.tsp.j2` around lines 73 - 88,
The child/camel derivation logic is duplicated (the computations that set child
and camel in the templated Keys alias block and again in the model fields), so
extract that logic into a reusable Jinja macro (e.g., a macro
compute_child_and_camel(name) placed near the top of
eng/semconv/templates/registry/qyl/semconv.g.tsp.j2) that returns or sets the
pascal/camel forms; then replace the inline sets (the lines that define child
and camel used for Keys aliases and the similar block used for model fields)
with calls to this macro (referencing the existing symbols child and camel where
used by alias generation and model field generation) to ensure a single source
of truth for the conversion logic.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Type mapping omits semconv template types.

The type switch handles primitives and arrays but doesn't account for semconv's template[type] syntax (e.g., template[string] for dynamic key patterns). These will silently fall through to string, losing type fidelity.

Proposed fix

 {% elif attr.type == 'int[]' %}
 {% set tsp_type = 'int64[]' %}
+{% elif attr.type is string and attr.type.startswith('template[') %}
+{# template types map to string - the template variable is metadata only #}
+{% set tsp_type = 'string' %}
 {% else %}
 {% set tsp_type = 'string' %}
 {% endif %}

Alternatively, emit a comment documenting the fallback is intentional for template types.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@eng/semconv/templates/registry/qyl/semconv.g.tsp.j2` around lines 138 - 150,
The template's type-mapping conditional (uses attr.type and sets tsp_type)
currently converts primitives and arrays but silently treats semconv
template[...] types as generic "string"; update the conditional in
eng/semconv/templates/registry/qyl/semconv.g.tsp.j2 to detect values matching
the pattern "template[...]" (e.g., using a startswith or regex check on
attr.type) and map them to an appropriate tsp_type (for example preserve
"template[string]" → "template[string]" or map to a distinct type like
"template[string]"/"string_map" per project convention), or if you intend to
keep the fallback, explicitly emit a comment in the template noting that
template[...] types intentionally collapse to "string" so reviewers/readers see
the behavior; ensure references to attr.type and tsp_type in the conditional are
updated accordingly.