Skip to content

Recorded the decision to warn when a tier resolves to no model - #53

Merged
dzykovic merged 2 commits into
mainfrom
fix/record-unknown-tier-decision
Oct 7, 2026
Merged

dzykovic merged 2 commits into
mainfrom
fix/record-unknown-tier-decision

Conversation

@dzykovic

@dzykovic dzykovic commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Summary

#49 shipped the owner's choice to warn — not refuse, not fall back to heavy — when an agent's tier resolves to no model. The reasoning lived only in that PR's body, which its review flagged in every round: a decision a later reader would otherwise re-litigate belongs in decisions/. This records it as decision 0016 and points the resolve_model_var comment at it. No behavior changes.

Type of change

  • CLI lifecycle or package behavior
  • Sync engine or adapter
  • Rule, agent, or skill content
  • Documentation
  • Refactor with no behavior change

Public CLI check

  • Uses only the established public command groups (no command changes)
  • Keeps preview/apply behavior explicit for planned writes (unchanged)
  • Covers automatic project alignment and CI refusal through the existing suites
  • Keeps fresh-clone package restoration behind intelligence sync (unchanged)

Verification

Read-only, in this PR's checkout at its head.

  1. Open decisions/0016-warn-when-a-tier-resolves-to-no-model.md. Expected: Status: accepted, with Context, Decision, Consequences and Rejected sections, and the decision matches the behavior Warned when an agent tier resolves to no model for a target #49 shipped — one WARNING: per tool and tier, an empty model still rendered, a manifest-defined tier quiet for its tool.
  2. Run grep -n "decision 0016" engine/lib/common.sh. Expected: one hit, in the comment above resolve_model_var.
  3. Run git diff origin/main...HEAD --stat. Expected: only the new decision file and a one-line comment change in engine/lib/common.sh.

Automated coverage

None needed: no executable line changes. bash cli/tests/verify.sh lint passed locally; CI runs the full suites on this head.

Versioned artifacts

  • No release state changes: a decision record and a comment need no changelog line
  • No version file moves
  • No CHANGELOG.md change
  • No stable release is published by this PR

Notes for reviewers

Other open branches also number new records 0015; this one takes 0016 to stay clear of #50. Whichever lands later renumbers.

Copilot AI balanced review requested due to automatic review settings October 7, 2026 12:54
@dzykovic dzykovic added the ai:processing An agent is working on this pull request label Oct 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The decision record contains an incomplete sentence and an inaccurate rationale for rejecting fallback behavior.

Review effort: Balanced
Findings: 2 Low severity

Open (2)
What changed in this PR

Records the rationale for warning when an agent tier resolves to no model, without changing runtime behavior.

Changes:

  • Adds decision 0016 documenting the warning policy and rejected alternatives.
  • Links resolve_model_var documentation to the decision.
File Description
engine/​lib/​common.sh References decision 0016 in an existing comment.
decisions/​0016-warn-when-a-tier-resolves-to-no-model.md Records the model-resolution warning decision.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread decisions/0016-warn-when-a-tier-resolves-to-no-model.md Outdated
Comment thread decisions/0016-warn-when-a-tier-resolves-to-no-model.md Outdated
@dzykovic

dzykovic commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Verification - PASS

head: 555a06b - steps: Manual Verification #1-3 (decision file content, grep for decision-0016 reference, diff --stat scope); no qa_checks matched (docs/qa-checks.md absent) - env: none (declared steps are read-only commands against the checkout; no app to bring up)

# Step Source Verdict Observed
1 Open decisions/0016-warn-when-a-tier-resolves-to-no-model.md. Expected: Status: accepted, with Context, Decision, Consequences and Rejected sections, and the decision matches the behavior #49 shipped — one WARNING: per tool and tier, an empty model still rendered, a manifest-defined tier quiet for its tool. PR pass File has Status: accepted (line 4) and ## Context, ## Decision, ## Consequences, ## Rejected headings. Cross-checked against engine/lib/common.sh's resolve_model_var/get_model (lines 1084-1155): a typo tier falls through to get_model, which returns empty when no override and no default exist, IS_MODEL is set (possibly empty) and a WARNING: is printed once per $IS_MODEL_IDE:$1 key via IS_MODEL_WARNED, deduplicated across agents in the run; when models.<ide>.<tier> is set, get_model returns the override and resolve_model_var returns before reaching the warning, i.e. quiet for that tool. All three behavior claims hold in the code.
2 Run grep -n "decision 0016" engine/lib/common.sh. Expected: one hit, in the comment above resolve_model_var. PR pass Output: 1135:# \WARNING:` lines (decision 0016).— exactly one hit, inside the comment block (lines 1130-1135) immediately aboveresolve_model_var` (defined line 1137).
3 Run git diff origin/main...HEAD --stat. Expected: only the new decision file and a one-line comment change in engine/lib/common.sh. PR pass Output: decisions/0016-warn-when-a-tier-resolves-to-no-model.md | 41 ++++++++ and engine/lib/common.sh | 2 +-, total 2 files changed, 42 insertions(+), 1 deletion(-). The common.sh delta is the single comment line changed (1 line replaced = 1 insertion + 1 deletion); no executable line changed.

Notes

  • Negative/boundary probe for step 1: re-read the implementation directly (not just the PR's description of it) to confirm the decision document's three behavioral claims are accurate, rather than trusting the prose — all three held against engine/lib/common.sh:1084-1155.
  • No standing checks applied: profile qa_checks defaults to auto → docs/qa-checks.md, which does not exist in this repository, so no area-keyed checks matched decisions/0016-...md or engine/lib/common.sh.
  • No environment was brought up: all three declared steps are read-only commands executed directly against the checkout at head (file read, grep, git diff); there is no application surface this change exercises, consistent with the PR's own "Read-only, in this PR's checkout at its head" framing.

@dzykovic dzykovic added the ai:verified Declared behavior verified on the reported head label Oct 7, 2026
@dzykovic

dzykovic commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Code review - CLEAN

head: 555a06b - base: main - files 2, +42/-1

Critical

(none)

Warning

(none)

Suggestion

(none)

Everything checks out. The diff is exactly the two files the PR body's Verification step 3 predicts: decisions/0016-warn-when-a-tier-resolves-to-no-model.md (new) and a one-line comment change in engine/lib/common.sh:1135. The comment's (decision 0016) reference is the single hit grep -n "decision 0016" engine/lib/common.sh finds, sitting directly above resolve_model_var as step 2 describes.

Decision 0016's Decision section matches the code it documents line for line: resolve_model_var (engine/lib/common.sh:1137-1155) renders an empty IS_MODEL and prints one unindented WARNING: no $IS_MODEL_IDE model for tier '$1' ... per tool/tier via the IS_MODEL_WARNED dedup set (:1146-1151), naming both the standard-tier list and the models.<tool>.<tier> override in the message text itself; a tier resolved by get_model (manifest override present) returns before the warning branch, so it stays quiet per tool as claimed. This is the behavior PR #49 (merged 7dfbb01) shipped and its own review flagged every round (d6634cc, 37b837d, 5aa3c9b) as missing a decision record, citing the same decisions/0013-add-a-frontier-model-tier.md precedent this PR now follows for shape (Context/Decision/Consequences/Rejected, Date/Status header). The Rejected section's three options line up with issue #39's "Decision needed" heading (fall back to heavy / warn and fall back / fail the sync) plus the status quo it closes.

No behavior changes, so no CHANGELOG line and no version-file move is needed, matching docs.md ("the why lives [in decisions/]") and release.md's lockstep requirement (which this diff doesn't touch). No migration, secret or environment change is in this diff, so no deployment-notes gap applies. The 0016 number intentionally skips the unmerged 0015 other open branches (and #50) are also using; the PR body's "Notes for reviewers" names that choice and that whichever lands later renumbers - no collision today since origin/main itself has no 0015 yet. Commit message ("Recorded the decision to warn when a tier resolves to no model") is a single, capitalized, past-tense line describing the one logical change, no attribution trailer. CI is green at this head except cli-e2e (macos-latest) still pending at review time - a verification-stage concern, not a code-review one.

@dzykovic dzykovic added the ai:reviewed Diff reviewed on the reported head label Oct 7, 2026
@dzykovic dzykovic removed ai:verified Declared behavior verified on the reported head ai:reviewed Diff reviewed on the reported head labels Oct 7, 2026
@dzykovic

dzykovic commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Verification - PASS

head: b9861f2 - steps: Manual Verification #1-3 (decision file content vs. shipped behavior, grep for decision-0016 reference, diff --stat scope); no qa_checks matched (docs/qa-checks.md absent) - env: none (declared steps are read-only commands against the checkout; no app surface to bring up)

# Step Source Verdict Observed
1 Open decisions/0016-warn-when-a-tier-resolves-to-no-model.md. Expected: Status: accepted, with Context, Decision, Consequences and Rejected sections, and the decision matches the behavior #49 shipped — one WARNING: per tool and tier, an empty model still rendered, a manifest-defined tier quiet for its tool. PR pass File has Status: accepted (line 4) and ## Context, ## Decision, ## Consequences, ## Rejected headings. Rather than infer the three behavioral claims from reading engine/lib/common.sh, sourced it and executed resolve_model_var directly: an unknown tier (haevy) on claude with no override printed one WARNING: no claude model for tier 'haevy' ... and left IS_MODEL empty; repeating the same tool+tier printed no second warning (dedup); the same tier on a different tool (codex) warned again (per-tool, not global); adding a models.claude.reviewdeep override and resolving that tier printed no warning and set IS_MODEL=opus-override. All three claims hold by observed execution, not just by reading the code.
2 Run grep -n "decision 0016" engine/lib/common.sh. Expected: one hit, in the comment above resolve_model_var. PR pass Output: 1135:# \WARNING:` lines (decision 0016).— exactly one hit, inside the comment block (lines 1130-1135) immediately aboveresolve_model_var` (defined line 1137).
3 Run git diff origin/main...HEAD --stat. Expected: only the new decision file and a one-line comment change in engine/lib/common.sh. PR pass Output: decisions/0016-warn-when-a-tier-resolves-to-no-model.md | 44 ++++... and engine/lib/common.sh | 2 +-, total 2 files changed, 45 insertions(+), 1 deletion(-). The common.sh delta is the single comment line changed; no executable line changed.

Notes

  • This head (b9861f29) differs from the previously-reported head (555a06bc) only in decisions/0016-...md prose (fallback-rationale and one unclear sentence corrected per git diff 555a06bc..b9861f29); engine/lib/common.sh is byte-identical to the already-verified commit. Re-executed all three steps fresh against this head rather than carry the prior report forward, since a push invalidates every factor.
  • Negative/boundary probe for step 1: actually ran the dedup and per-tool/per-tier boundaries (repeat call, different tool, manifest override) instead of trusting the prose or the code reading alone — all four cases observed as the decision document states.
  • No standing checks applied: profile qa_checks defaults to auto → docs/qa-checks.md, absent in this repository (checked in-worktree), so no area-keyed checks matched decisions/0016-...md or engine/lib/common.sh.
  • No environment was brought up: all three declared steps are read-only commands run directly against the checkout at head (file read, grep, git diff, plus a sourced-function probe that modified no repository file); consistent with the PR body's own "Read-only, in this PR's checkout at its head" framing. Worktree remained clean and at b9861f29639827d1a44502c38fe5d0e5f05b732c throughout.

@dzykovic dzykovic added the ai:verified Declared behavior verified on the reported head label Oct 7, 2026
@dzykovic

dzykovic commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Code review - CLEAN

head: b9861f2 - base: main - files 2, +45/-1

Critical

(none)

Warning

(none)

Suggestion

  • decisions/0016-warn-when-a-tier-resolves-to-no-model.md:40-43 - the corrected "Falling back to heavy" rationale ("costlier than a misspelled light or standard meant, weaker than a misspelled frontier") holds for the tools with four distinct model defaults (claude, codex, copilot, opencode) but not for cursor, where every tier maps to inherit (engine/lib/common.sh:993-996), or antigravity, where frontier and heavy both map to pro (engine/lib/common.sh:1009-1010). Consider scoping the claim to the tools it actually holds for, or naming the exception the way decision 0013's Consequences section already does for tiers that render identical output.

This commit (b9861f2) does exactly what its message says: it corrects the Context clause ("A tier that neither source defines") and the Rejected section's heavy-fallback rationale, nothing else. The diff is confined to decisions/0016-warn-when-a-tier-resolves-to-no-model.md (new, 44 lines) and the single (decision 0016) comment addition on engine/lib/common.sh:1135, matching the PR body's Verification step 3 (git diff origin/main...HEAD --stat → 2 files, +45/-1).

Decision 0016 documents the shipped behavior accurately: resolve_model_var (engine/lib/common.sh:1137-1155) renders an empty IS_MODEL and warns once per $IS_MODEL_IDE:$1 via the IS_MODEL_WARNED dedup set (Decision §1); the line is an unindented WARNING: so sync --compact keeps it (Decision §2 - the exact comment this PR's diff touches); get_model returns a manifest override before the warning branch is reached, so an overridden tier stays quiet for that tool (Decision §3). The Context's issue-#39 citation and the Rejected section's three options still mirror that issue's "fall back to heavy / warn and fall back / fail the sync" framing and PR #49's own "Notes for reviewers" rejection of refusing the sync. Shape (Date/Status header, Context/Decision/Consequences/Rejected) matches sibling decisions/0013-add-a-frontier-model-tier.md and the template in decisions/README.md.

No migration, secret or environment change is in this diff, so no deployment-notes gap applies; the PR body's "Versioned artifacts" checklist reflects that correctly. No executable line changed, so the new-logic-needs-tests gate (dev-verification-gates) doesn't apply either. Both commits (555a06b, b9861f2) are capitalized, past-tense, one logical change each, no attribution trailer - consistent with git-commit-conventions.

@dzykovic dzykovic added ai:reviewed Diff reviewed on the reported head ai:completed Agent work completed; required factors are fresh and removed ai:processing An agent is working on this pull request labels Oct 7, 2026
@dzykovic
dzykovic merged commit b6c93ba into main Oct 7, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai:completed Agent work completed; required factors are fresh ai:reviewed Diff reviewed on the reported head ai:verified Declared behavior verified on the reported head

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants