Skip to content

docs(#7035): document provider-qualification invariant for model alias values - #7036

Open
fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/7035-document-model-alias-patterns
Open

docs(#7035): document provider-qualification invariant for model alias values#7036
fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/7035-document-model-alias-patterns

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

Documents the provider-qualification invariant for model alias values in the pi runtime internals section of docs/contributing/runtime-implementation.md. This prevents recurrence of the double-prefixing bug fixed in PR #7028, where piAgentModels blindly prepended anthropic-vertex/ to already-qualified alias values.

Changes

  • Added a "Provider-qualification invariant for model alias values" subsection to the pi runtime internals, covering:
    • The three alias value formats (bare model ids, provider-qualified ids, xai-vertex specs) and how each must be handled
    • References to translatePiModel and normalizeXaiVertexModel as canonical implementations of the three-way dispatch
    • A checklist for writing or modifying alias-transforming code, including the requirement for tests covering all three formats
  • Updated the AGENTS.md table entry for runtime-implementation.md to include a direct link to the new section, making it discoverable for future contributors and agents

Testing

  • make lint-md-links passes — all markdown links (including the new anchor) resolve correctly
  • No trailing whitespace or end-of-file issues
  • Secret scan passes
  • gitlint passes

Closes #7035

Post-script verification

  • Branch is not main/master (agent/7035-document-model-alias-patterns)
  • Secret scan passed (gitleaks — 6d496c4ce54bcccefdcbd5bec1ab44f775a33de8..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

…s values

Add a subsection to the pi runtime internals in runtime-implementation.md
documenting the three-way dispatch pattern that any code transforming
model alias values must follow. The section explains the three alias
value formats (bare ids, provider-qualified ids, xai-vertex specs),
references translatePiModel and normalizeXaiVertexModel as canonical
implementations, states the invariant, and notes that tests must cover
all three formats. This prevents recurrence of the double-prefixing bug
fixed in PR #7028, where piAgentModels blindly prepended a provider
prefix to already-qualified values.

Also updates the AGENTS.md table entry for runtime-implementation.md
to make the new section discoverable.

Closes #7035
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner September 5, 2026 01:18
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Sep 5, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 5, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:20 AM UTC · Completed 1:39 AM UTC

Commit: 324fcad · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.49

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Site preview

Preview: https://8c5aeb7a-site.fullsend-ai.workers.dev

Commit: d4486bbb14b2f4cab3a16b2f750d7ff16de94b46

@codecov

codecov Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Sep 5, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 5, 2026

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Small documentation-only PR (2 files, 57 lines) from a known bot author; elevated above low by high file churn (49.5 avg commits/30d) and multi-author contention (9 authors/90d) on the changed files, but mitigated by small scope, low-priority linked issue, no security/dependency impact, and trivial rollback safety.

Previous run

Risk Assessment: moderate (2/5)

Details

Small documentation-only PR (2 files, 57 lines) from a known bot author adding provider-qualification invariant docs; elevated above low by high file churn (37-60 commits/30d) and multi-author contention on the changed files, but mitigated by the small scope, low-priority linked issue, no security/dependency impact, and trivial rollback safety.

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 5, 2026

Copy link
Copy Markdown

Review

Findings

Medium

Previous run

Review

Findings

Medium

Low

  • [internal-consistency] docs/contributing/runtime-implementation.md:619 — The statement "piAgentModels builds the child model table and must apply the same three-way dispatch independently — it cannot reuse translatePiModel" is imprecise: piAgentModels DOES call translatePiModel for the "default" entry (pi_bootstrap.go line 486: "default": translatePiModel(base, configAliases)). The restriction applies only to the alias resolution loop (lines 487–499), where each alias value is dispatched independently using piDefaultProvider instead of the env var. A developer following the recipe could misunderstand the blanket "cannot reuse" as prohibiting any call to translatePiModel.
    Remediation: Qualify the statement, e.g. "the alias resolution loop in piAgentModels applies the same three-way dispatch independently — it cannot reuse translatePiModel for alias entries because that reads FULLSEND_PI_PROVIDER".

  • [reference-format] docs/contributing/runtime-implementation.md:617 — The reference PR #7028 uses a PR prefix that appears nowhere else in the document. The file has 34 bare #NNNN references across 19 distinct issue/PR numbers and zero instances of the PR #NNNN form. The established convention is the bare #7028.
    Remediation: Change PR #7028 to #7028.

  • [table-cell-convention] AGENTS.md:36 — The "When to read" column now contains an inline Markdown link ([provider-qualification invariant](docs/contributing/runtime-implementation.md#...)) while every other row in the table uses only plain text with optional backtick code identifiers — no other row links to a subsection from inside the description cell.
    Remediation: Use plain text instead: the provider-qualification invariant for model alias values (the three alias value formats code must handle).


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 1:41 AM UTC · Completed 1:46 AM UTC

Commit: 324fcad · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.06

- Qualify the `piAgentModels` / `translatePiModel` statement to clarify the
  restriction applies to the alias resolution loop, not the "default" entry
- Change `PR #7028` to bare `#7028` to match the document's reference convention
- Remove inline Markdown link from AGENTS.md table cell to follow the
  plain-text convention used by all other rows

Addresses #7036
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (bot-triggered)

Addressed all four review findings: qualified the imprecise piAgentModels/translatePiModel statement, normalized the PR #7028 reference to bare #7028 format, removed the inline Markdown link from the AGENTS.md table cell, and recorded reasoned disagreement on the protected-path informational finding.

Fixed (3):

  1. piAgentModels statement is imprecise — it DOES call translatePiModel for the default entry (docs/contributing/runtime-implementation.md): Qualified the statement to clarify the restriction applies to the alias resolution loop, not the default entry: 'The alias resolution loop in piAgentModels applies the same three-way dispatch independently — it cannot reuse translatePiModel for alias entries'
  2. PR fix(#7020): point fable at claude-fable-5-1 and thread config aliases into sub-agent dispatch #7028 uses a PR prefix that appears nowhere else in the document (docs/contributing/runtime-implementation.md): Changed 'PR fix(#7020): point fable at claude-fable-5-1 and thread config aliases into sub-agent dispatch #7028' to bare 'fix(#7020): point fable at claude-fable-5-1 and thread config aliases into sub-agent dispatch #7028' to match the document's established reference convention (34 bare #NNNN references, zero PR #NNNN instances)
  3. AGENTS.md table cell contains an inline Markdown link breaking the plain-text convention (AGENTS.md): Removed the inline Markdown link from the 'When to read' column, replacing with plain text to match every other row in the table

Disagreed (1):

  1. AGENTS.md is a protected governance file requiring human approval: The change to AGENTS.md is part of the PR's original scope (making the new section discoverable from the index table). The review correctly notes human approval is always required for protected-path changes — this is an informational finding, not a code defect. The table-cell-convention fix below addresses the only actionable concern in this file.

Tests: passed

Decision points
  • Treated the protected-path finding as informational rather than actionable (alternatives: Revert the AGENTS.md change entirely, Keep the change with the convention fix; rationale: The AGENTS.md change is intentional PR scope (making the new section discoverable). The review finding correctly notes human approval is required but does not request reverting the change. The table-cell-convention fix addresses the only actionable concern.)

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 5, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:48 AM UTC · Completed 2:06 AM UTC

Commit: d4486bb · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.58

@fullsend-ai-review
fullsend-ai-review Bot dismissed their stale review September 5, 2026 02:06

Superseded by updated review

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-review Triggers review agent dispatch requires-manual-review Review requires human judgment risk/moderate PR risk: moderate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Document provider-qualification patterns for model alias resolution in runtime docs

0 participants