Skip to content

chore: bump protoc 6.31.1→6.33.2; remove superseded Task03 TDD script - #71

Closed
Ken Chau (kenotron-ms) wants to merge 1 commit into
mainfrom
chore/proto-cleanup
Closed

chore: bump protoc 6.31.1→6.33.2; remove superseded Task03 TDD script#71
Ken Chau (kenotron-ms) wants to merge 1 commit into
mainfrom
chore/proto-cleanup

Conversation

@kenotron-ms

Copy link
Copy Markdown
Contributor

Split from #70 so each diff is reviewable independently.

## Changes

- `proto/amplifier_module_pb2.py`: header-only bump from protoc 6.31.1 → 6.33.2 (no schema changes)
- `proto/test_task03_module_specific.py`: delete 239-line TDD script from Task03 planning cycle — used `subprocess` + `protoc` binary (fragile), tested enum/field counts now covered by Rust prost compilation, never wired into pytest. Superseded.

## Why separate?

Per code review feedback on #70: unrelated to the `cost_usd` field addition. Splitting keeps each diff reviewable on its own merits.

… Task03 TDD script

    proto/amplifier_module_pb2.py: regenerated with protoc 6.33.2 — header-only change,
    no schema content altered.

    proto/test_task03_module_specific.py: deletes a 239-line TDD validation script from
    the Task03 planning cycle. It ran via __main__ using subprocess+protoc (fragile),
    tested enum/field counts already covered by Rust prost compilation, and was never
    wired into pytest. Superseded. Not related to cost_usd.

    Splitting from feat/m1-cost-usd-upstream so each diff is reviewable on its own merits.

    🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

    Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Ken Chau (kenotron-ms) added a commit that referenced this pull request May 5, 2026
…rage

- proto/amplifier_module_pb2.py and proto/test_task03_module_specific.py
  reverted to main state — the protoc header bump (6.31.1→6.33.2) and
  the Task03 TDD script deletion are unrelated to cost_usd and have been
  split to PR #71 so each diff is reviewable on its own merits.

- scripts/bump_version.py: add python/amplifier_core/__init__.py to
  VERSION_FILES and extend VERSION_LINE_RE to match `__version__ = "X.Y.Z"`
  (was matching only `version = "X.Y.Z"`). This was the root cause of the
  1.0.7 version lag on main — the bump script didn't cover __init__.py.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Salil Das (sadlilas) pushed a commit that referenced this pull request May 6, 2026
)

* feat(core): add cost_usd: Decimal | None to Usage model

Declared field (not extras bag) so Pydantic enforces the type.
Float input is rejected with an explicit error message.
None = unknown cost, Decimal('0') = explicitly free.
Serializes as string in JSON mode for safe transport.

* test(core): tighten TestUsageCostUsd — remove speculative imports, fix string test, narrow exception

* feat(core): add cost_usd: Decimal to SessionStatus; deprecate estimated_cost: float

cost_usd is the canonical accumulated session cost.
estimated_cost kept with deprecated=True for backward compat.
Float input rejected with explicit validation error.

* feat(core): add cost_usd: Option<String> to Rust SessionStatus

String type matches Decimal's JSON serialization convention.
estimated_cost: Option<f64> retained for backward compat.

Also adds TestSchemaSync Python tests verifying that the Pydantic
model schema includes cost_usd with string-compatible type.

* feat(core): add cost_usd: optional string to proto Usage message

Field 7. String matches Decimal JSON serialization.
None = unknown cost.

Proto stubs regenerated with grpc_tools.protoc.

* fix: add cost_usd field to generated Usage conversions and equivalence test

The proto Usage message gained cost_usd: Option<String> in amplifier.module.rs,
but the generated conversion and test files were not updated.

- conversions.rs: set cost_usd: None when converting messages::Usage → proto Usage
  (messages::Usage has no cost_usd field; cost is tracked at session level)
- equivalence_tests.rs: add cost_usd: Some("0.001234".into()) to the
  proto_usage_has_all_token_fields test and corresponding assertion

Fixes Rust compilation error: E0063 missing field cost_usd

* chore: bump version to 1.4.0 for M1 cost_usd fields

* fix: address COE review — remove estimated_cost, fix proto conversion, bump 1.5.0

- Remove estimated_cost: float from SessionStatus (Python + Rust) — was never
  populated, removal was always the plan per M1 design
- Fix proto cost_usd field: messages::Usage has no cost_usd, so removed
  optional string cost_usd = 7 from proto entirely; regenerated pb2.py and
  amplifier.module.rs (prost also added Copy derive to Usage now all fields
  are Copy-safe)
- Remove cost_usd: None from conversions.rs From<messages::Usage> impl
- Remove cost_usd references from equivalence_tests.rs
- Delete proto/test_task03_module_specific.py (tested cost_usd field that
  no longer exists in proto)
- Bump version to 1.5.0 (upstream was already at 1.4.1; 1.5.0 is correct
  next MINOR)
- Add Field(description=...) to Usage.cost_usd in message_models.py for
  IDE/schema visibility
- Add EOF newline to models.rs

* docs: document String type choice for cost_usd — no rust_decimal dependency

The kernel stores cost_usd as Option<String> rather than
rust_decimal::Decimal deliberately:
- kernel does no arithmetic on cost (transport-only field)
- type enforcement (Decimal, float rejection) belongs in Python
- avoids rust_decimal dependency in the kernel

If cost arithmetic ever moves into the kernel, the type must change.

* style: cargo fmt — split long string literal in test

Rustfmt CI requires lines under the column limit.
The long raw string in session_status_cost_usd_roundtrip
was split across two lines to satisfy the formatter.

* fix: split proto cleanup to #71; fix bump_version.py __init__.py coverage

- proto/amplifier_module_pb2.py and proto/test_task03_module_specific.py
  reverted to main state — the protoc header bump (6.31.1→6.33.2) and
  the Task03 TDD script deletion are unrelated to cost_usd and have been
  split to PR #71 so each diff is reviewable on its own merits.

- scripts/bump_version.py: add python/amplifier_core/__init__.py to
  VERSION_FILES and extend VERSION_LINE_RE to match `__version__ = "X.Y.Z"`
  (was matching only `version = "X.Y.Z"`). This was the root cause of the
  1.0.7 version lag on main — the bump script didn't cover __init__.py.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>

---------

Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant