feat: add AWS Bedrock provider transport - #415
Open
sairam0424 wants to merge 5 commits into
Open
Conversation
Bedrock over SigV4 is not OpenAI-compatible, so it cannot be added as just
another base_url. This introduces an explicit `transport` discriminator in
providers.json alongside a BedrockConverseProvider, and keeps every existing
provider working by defaulting `transport` to "openai".
Why a new transport rather than a base_url:
- auth is SigV4 from the AWS credential chain, not a bearer token
- `system` is a top-level Converse parameter, not a message role
- there is no `response_format`; structured output requires declaring a tool
whose inputSchema.json is the caller's schema and forcing toolChoice
Both providers return {"message": {"content": ...}}, so pdf.py, evaluator.py and
github.py are untouched by the transport choice. In structured mode the tool
payload is re-serialized to a JSON string so extract_json_from_response and the
existing json.loads call sites keep working unchanged.
Two capability facts, both established against the live API rather than assumed:
- structured_output is now resolvable per model, not only per provider.
Claude and Nova honour a forced toolConfig; google.gemma-3-* ignores it and
answers in prose with no error, so those models declare "none". Declaring
capability per provider would silently degrade them.
- Bedrock's Anthropic models reject temperature and top_p together with a
ValidationException. pdf.py:94 read model_params["top_p"] with bracket
access and evaluator.py:66 defaulted it to 0.9, so both forced the pair in
and every Bedrock Anthropic call failed. Both now spread the resolved
params, so a model that declares one sampling parameter sends one. This is
a latent bug in the existing code that any single-parameter model would
have hit.
Also fixes .gitignore, which matched `test_*.py` at any depth and therefore made
tests/ uncommittable. Narrowed to `/test_*.py` to preserve the original intent
of ignoring stray root-level scratch files.
Verified: 20 offline tests pass, and a full end-to-end
`score.py <resume> --role software_engineering_intern` run completes against
us.anthropic.claude-haiku-4-5-20251001-v1:0 and amazon.nova-pro-v1:0 with
schema-conforming output.
…irks
Adds claude-opus-5, claude-sonnet-5, claude-opus-4-8, claude-sonnet-4-6 and
claude-sonnet-4-5 to the bedrock provider, plus a measurement script that picks
between them on evidence instead of spec sheets.
Sampling parameters turn out to have three regimes within this one provider,
each enforced by Bedrock with a ValidationException:
- nova / qwen / gemma temperature + top_p
- haiku-4-5, sonnet-4-5/4-6 temperature only
- opus-5, sonnet-5, opus-4-8 neither; both are deprecated
Models in the third group therefore declare {} and rely on the params being
spread rather than named. A consequence worth knowing: temperature=0 is not
available on the newest models, so it cannot be used to damp score variance.
Tool-input normalization. Forcing toolChoice guarantees a toolUse block but not
a well-formed one. Two quirks were captured against the live API while scoring a
real resume, and in both the model's content was correct and only its envelope
was wrong, so repair is lossless rather than a guess:
1. Envelope wrapping. opus-5 and sonnet-5 nest the entire object under one
arbitrary key -- {"parameter_name": {...}}, {"response": {...}}, and the
literal placeholder {"$PARAMETER_NAME": {...}}. Unwrapped only when that
lone key is not itself a schema property and the nested dict does contain
schema properties, so a genuine single-property response is untouched.
2. Stringified nested values. haiku-4-5 emits `scores` and `bonus_points` as
JSON strings, each with one spurious trailing brace, so json.loads rejects
an otherwise complete object. Parsed with raw_decode and accepted only when
the trailing remainder is pure punctuation noise; if any real content would
be dropped the string is left as-is so validation fails loudly rather than
silently recording a partial score.
Only strings beginning with { or [ are considered, so prose fields such as
`evidence` and `breakdown` are never reinterpreted.
Measured effect, 6 runs of one resume through the scoring call:
before after
opus-5 1/5 6/6
sonnet-5 1/5 4/6
sonnet-4-6 5/5 6/6
haiku-4-5 1/5 2/6
The first comparison run was measuring this serialization gap rather than model
reliability. haiku-4-5 remains unfit for this schema and the README says so.
29 tests, including both quirks asserted against the exact payloads captured
from the API, and a case proving real trailing content is refused.
…uesses
criteria.jinja:60 makes "all projects are self_project" a hard 10-point ceiling on
open_source and system_message.jinja:23/49 tells the model to read project_type.
github.py computes that field at the cost of one contributors call per repository.
It never reached the prompt, so the rule was decided by hallucination.
Observed on a real resume before this change: the pipeline printed
"12 open source, 20 self projects" and the model then asserted "all 7 featured
projects are self-projects (single contributor, 0 forks each)" and scored
open_source 12/35 on that basis. `grep -c project_type transform.py` was 0.
Two separate defects, both fixed:
1. The selection step discarded the computed fields. generate_projects_json
appended the model's own echoed dict, which omits project_type and
contributor_count entirely and re-types stars, forks and commit counts from
memory — so a hallucinated number would enter the scoring prompt as if it had
been fetched. Extracted resolve_selected_projects(), which keeps the model's
choice, ordering and stated rationale but takes every factual field from the
fetched record, and drops a selection naming a repository that was never
fetched. Note the two exception fallbacks were already doing this correctly;
only the happy path was wrong.
2. convert_github_data_to_text omitted the fields even when present. Now emits
project_type, contributor_count, and author/total commit counts, plus live_url
— a working demo is the largest single modifier in the rubric (+10-20% on
self_projects, and -2 to -3 per project without one), and it was being fetched
and then dropped. Missing values render as "unknown"/"N/A" rather than
vanishing, so a cache written before this change is still legible.
Measured effect, claude-opus-5, scoring call only, same cached extraction:
open_source total
before 12 [12-12] ±0 74 [71-76] ±5 (6 runs)
after 16 [16-16] ±0 82 [78-82] ±4 (5 runs)
±0 spread on both sides, so the +4 is a deterministic consequence of the model
receiving real data rather than run-to-run variance. The model's reasoning also
became checkable: it now writes "despite several repos being marked
multi-contributor, the candidate authored ~95-99% of commits in each" instead of
asserting single-contributor status it had no data for.
16 new tests: 8 covering serializer output, 8 covering resolution integrity
including hallucinated-fact rejection, invented-project rejection, order
preservation and non-mutation of the fetched records.
A clean `pip install -r requirements.txt` followed by `pytest tests/` failed with ModuleNotFoundError: ho module named 'pytest' -- it only worked in the maintainer's local venv because pytest was installed there manually, outside the dependency manifest. Pinned to 9.1.1, the version already verified against all 45 tests in this PR.
…exist The module docstring pointed to test_bedrock_live.py, which was never added to the repo.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
BedrockConverseProvider(models.py) as a new LLM transport wired throughproviders.json/config.py/llm_utils.py, so any configured model can route through AWS Bedrock's Converse API (SigV4 auth via boto3's standard credential chain) alongside the existing OpenAI-compatible transport.transportdefaults to"openai"when omitted, so all existing providers are unaffected.github.py/transform.py):resolve_selected_projectsno longer trusts the LLM's echoed project-selection JSON, which was silently droppingproject_typeandcontributor_countand could re-type commit/star counts from memory. Selected projects are now re-keyed from the authoritative fetched GitHub data, keeping only the model's ownreason_for_project_selection.pytestdeclared as a dependency so the suite runs immediately after a clean install.Testing
Full test suite:
pytest -v→ 45 passed, 0 failed, 0 skipped, 5 warnings in 0.45s. Breakdown:tests/test_bedrock_provider.py(29 tests),tests/test_github_signal_integrity.py(8 tests),tests/test_project_resolution.py(8 tests). This is the repo's entire test suite —mainhas zero pre-existing tests, so there is no regression risk to any prior suite. The 5 warnings are pre-existing, unrelated PyMuPDF/SWIGDeprecationWarnings.Live AWS Bedrock verification across all 3 sampling-parameter regimes — real Converse API calls via boto3 (SigV4, no mocking), driven through the actual production call path (
initialize_llm_provider(model).chat(...)):temperature+top_pregime →amazon.nova-pro-v1:0: clean structured-output parse in 1.71s.temperature-only regime →us.anthropic.claude-sonnet-4-6: clean structured-output parse in 4.22s.{}) regime →us.anthropic.claude-sonnet-5: clean structured-output parse in 4.08s.Project-metadata fix verified live, end-to-end: ran
score.py <resume.pdf> --role software_engineering_internthrough the full pipeline against real, previously-fetched GitHub data for 7 repositories. The returned evidence text cites the corrected computed signals directly rather than guessing — e.g. quoting real commit counts per repository and reasoning about contributor counts and project type — exactly the fieldsresolve_selected_projectsnow restores from the authoritative fetched record instead of trusting the LLM's own echoed (and bug-prone) selection JSON.Fresh-clone install verification, cold: in an isolated git worktree, built a brand-new Python 3.11 virtualenv and ran
pip install -r requirements.txtfrom a clean slate — exit code 0, zero errors. Then ranpytest tests/immediately, with no manual package installation of any kind — 45 passed, 5 warnings in 2.74s. This confirms a first-time contributor or CI runner following only the README's own instructions can install and run the full suite, fully offline (the Bedrock tests mock the Converse client; no AWS credentials or.envfile required).