Skip to content

fix: SLI selftest SLI_SELFTEST_DIR 缺省(.github #98,ADR-0059) - #194

Merged
randypanding merged 1 commit into
mainfrom
p34/sli-fix1
Aug 21, 2026
Merged

fix: SLI selftest SLI_SELFTEST_DIR 缺省(.github #98,ADR-0059)#194
randypanding merged 1 commit into
mainfrom
p34/sli-fix1

Conversation

@randypanding

Copy link
Copy Markdown
Contributor

T4 首跑暴露:workflow 自测步骤未传 SLI_SELFTEST_DIR,set -u 下 unbound。缺省点号。

Copilot AI lite review requested due to automatic review settings August 21, 2026 13:06
@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@randypanding, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 4 minutes

Limit details: You’ve used all 10 included reviews currently available.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9f6646d5-34d0-4562-a33e-86d63f0b8638

📥 Commits

Reviewing files that changed from the base of the PR and between c5a6317 and b2d39ab.

📒 Files selected for processing (1)
  • scripts/sli-report.sh

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix sli-report self-test default for SLI_SELFTEST_DIR under set -u

🐞 Bug fix 🕐 Less than 5 minutes

Grey Divider

AI Description

• Default SLI_SELFTEST_DIR to "." during self-test to avoid unbound failures under set -u.
• Keep existing self-test behavior while making first-run workflow execution reliable.
Diagram

graph TD
  A["CI workflow"] --> B{{"SLI_SELFTEST_DIR set?"}} -->|"yes"| C["scripts/sli-report.sh"] --> D["python3 calc"]
  B -->|"no (default '.')"| C

  subgraph Legend
    direction LR
    _ci["CI step"] ~~~ _dec{{"Decision"}} ~~~ _script["Script"] ~~~ _proc["Process"]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Early default assignment
  • ➕ Centralizes defaults near script entrypoint
  • ➕ Avoids repeating parameter expansion at each use site
  • ➖ Slightly more intrusive change if the variable is intentionally unset in other modes
  • ➖ Requires auditing all branches/flags for correct semantics
2. Fail fast with clearer error
  • ➕ Makes missing configuration explicit rather than silently defaulting
  • ➕ Avoids potentially masking misconfiguration
  • ➖ Does not solve the 'first run' self-test friction described in the PR
  • ➖ Adds churn to workflows calling the script

Recommendation: The current approach (using ${SLI_SELFTEST_DIR:-.} at the self-test use site) is a low-risk, minimal fix that directly addresses set -u unbound failures while preserving existing behavior. An early default assignment could be considered later if more call sites require the same default.

Files changed (1) +1 / -1

Bug fix (1) +1 / -1
sli-report.shDefault SLI_SELFTEST_DIR to '.' for self-test python invocation +1/-1

Default SLI_SELFTEST_DIR to '.' for self-test python invocation

• Updates the self-test path to pass '${SLI_SELFTEST_DIR:-.}' into the inline Python runner. This prevents 'set -u' from failing when the workflow doesn't provide SLI_SELFTEST_DIR on first run.

scripts/sli-report.sh

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.

Pull request overview

This PR fixes the --self-test path in scripts/sli-report.sh so it won’t fail under set -u when SLI_SELFTEST_DIR is unset, aligning with the reported first-run failure in .github #98 / ADR-0059.

Changes:

  • Default SLI_SELFTEST_DIR to . when invoking the embedded Python self-test helper, preventing an “unbound variable” error under set -u.

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

@randypanding
randypanding merged commit a79b8fd into main Aug 21, 2026
11 checks passed
@randypanding
randypanding deleted the p34/sli-fix1 branch August 21, 2026 13:08
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

randypanding pushed a commit that referenced this pull request Aug 21, 2026
…冲突解为 §16→§17(本卡)→§18 段序(§18 头注释已预留 §17 编号给 W1-C3);REPOS.yaml 保留 archive/holdout 新条目+entry_protocol 申报
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.

2 participants