Repository navigation
feat(settings-advisor): enhance scan detection and adopt traits-driven model selection - #20
Conversation
…n model selection
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33656d9ca8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ".html": true, | ||
| ".css": true, | ||
| ".sh": true, | ||
| ".dart": true, |
There was a problem hiding this comment.
Rebuild the bundled binaries for the changed scanner
Installed users execute the committed native binary through gemini-extension.json, but this commit changes main.go without updating any of the three native binaries. I confirmed that the bundled Linux binary still reports a Dart-only workspace as 0 lines, treats .env.example as a real environment file, and misses GitLab CI, so the documented scanning enhancements are not delivered by a normal plugin installation. Rebuild and commit every distributed platform binary.
AGENTS.md reference: AGENTS.md:L46-L48
Useful? React with 👍 / 👎.
| "target": true, | ||
| ".next": true, | ||
| ".nuxt": true, | ||
| "out": true, | ||
| ".dart_tool": true, | ||
| "Pods": true, |
There was a problem hiding this comment.
Do not skip the requested scan root
When the explicitly supplied repository path itself is named one of the newly added entries such as target, out, or Pods, filepath.Walk invokes this branch for the root and immediately returns filepath.SkipDir, producing an empty scan. The skip check should apply only to descendant directories (for example, when path != root) so arbitrary valid repository paths remain scannable, with a path-boundary regression test.
AGENTS.md reference: AGENTS.md:L42-L42
Useful? React with 👍 / 👎.
| if strings.Contains(slashPath, ".github/workflows") || | ||
| strings.Contains(slashPath, ".gitlab/ci") || | ||
| strings.Contains(slashPath, ".circleci") { |
There was a problem hiding this comment.
Match CI directories on path-component boundaries
For workspaces containing paths such as .circleci-disabled/job.yml or .gitlab/circle/config.yml, these substring checks set HasCI even though neither path is one of the supported CI directories, causing an unwarranted proceed-in-sandbox recommendation. Compare normalized path components, or require a separator boundary after .circleci and .gitlab/ci, and cover near-matching directory names in tests.
AGENTS.md reference: AGENTS.md:L42-L42
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51a6e34ce7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| dir := filepath.Dir(slashPath) | ||
| for _, part := range strings.Split(dir, "/") { | ||
| if hasProdToken(strings.ToLower(part)) { |
There was a problem hiding this comment.
Limit production matching to paths beneath the scan root
When an absolute scan root has an ancestor named prod or production (for example, /mnt/production/repos/app), slashPath includes that out-of-workspace component, so any JSON/YAML/TOML file marks HasProdConfig and causes an unwarranted strict permission recommendation. Derive the directory from a path relative to root before checking its components, and cover this ancestor-boundary case.
AGENTS.md reference: AGENTS.md:L41-L42
Useful? React with 👍 / 👎.
Address Codex review findings on PR #20. - skipDirs no longer skips the explicitly requested scan root, so a repository named `target` / `out` / `Pods` is still scannable. - CI and production-config detection now run off root-relative path components instead of substring matches on the absolute path. This fixes two faces of the same bug: `.circleci-disabled/` and `.gitlab/circle/` no longer set HasCI, and an out-of-workspace ancestor such as `/mnt/production/repos/app` or `~/.circleci/...` no longer sets HasProdConfig / HasCI. - Regression tests for all three cases (each verified to fail against the pre-fix scanner), plus a no-regression test covering real CI directory layouts. - Rebuilt the three distributed binaries with the pinned go1.26.5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Codex レビューの4件すべてに対応しました( P1 — Rebuild the bundled binaries: 対応済みです。指摘のレビュー対象コミット P2 — Do not skip the requested scan root: P2 — Match CI directories on path-component boundaries / P2 — Limit production matching to paths beneath the scan root: この2件は同一バグの別断面だったため、まとめて直しました。 テスト: 以下を追加し、いずれも修正前のスキャナに対して FAIL することを確認済みです(通過してしまう見せかけのテストでないことの確認)。
加えて @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 986d6a645c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| suffix := strings.TrimPrefix(fileName, ".env.") | ||
| switch suffix { | ||
| case "example", "sample", "template", "dist", "test", "defaults", "schema": |
There was a problem hiding this comment.
Ignore qualified environment templates
When a workspace contains a common qualified template such as .env.production.example or .env.local.sample, suffix becomes production.example or local.sample, so this exact-value switch misses it and returns true. This incorrectly sets HasEnv and recommends enabling the sandbox despite the documented template exclusion; recognize terminal template markers while preserving actual files such as .env.production, and cover qualified templates in the filter test.
Useful? React with 👍 / 👎.
`isEnvFile` compared the whole suffix against an exact set, so a common qualified template such as `.env.production.example` or `.env.local.sample` fell through and set HasEnv, recommending the sandbox despite the documented template exclusion. Match the terminal dot-separated token instead. This catches qualified templates while keeping files that hold real values, including `.env.production` and `.env.test.local`. Tests cover both forms; both new tests were verified to fail against the previous implementation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
P2「Ignore qualified environment templates」に対応しました( 指摘どおり 実値を持つファイルは残す必要があるので、境界はこう置きました。
テストは バイナリも |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0ea9d9573
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "sample": true, | ||
| "template": true, | ||
| "dist": true, | ||
| "test": true, |
There was a problem hiding this comment.
Treat
.env.test as an actual environment file
When a repository stores test-environment variables in .env.test, this marker makes isEnvFile return false, so HasEnv remains false and the sandbox recommendation is suppressed. .env.test commonly denotes an environment-specific configuration rather than a template; only unambiguous template suffixes such as example, sample, and template should be excluded.
Useful? React with 👍 / 👎.
| ".tf": true, | ||
| } | ||
|
|
||
| var skipDirs = map[string]bool{ |
There was a problem hiding this comment.
Skip Terraform and Swift dependency caches
When scanning an initialized Terraform or Swift workspace, .terraform/modules and SwiftPM's .build/checkouts contain dependency source with the newly counted .tf and .swift extensions, but neither directory is in this skip map. Those vendored files are therefore included in TotalLines and can push a small workspace into the mid or heavy tier despite the advertised package/build exclusion; add these standard cache directories and path regression tests.
AGENTS.md reference: AGENTS.md:L40-L42
Useful? React with 👍 / 👎.
Summary
Enhance workspace scan detection accuracy and transition model recommendation logic to a data-driven (traits-based) design for
settings-advisor.1. Enhanced Workspace Scanning
.gitlab-ci.yml,.gitlab/ci/), CircleCI (.circleci), Bitbucket (bitbucket-pipelines.yml), and Azure Pipelines (azure-pipelines.yml) alongside GitHub Actions..dart), Swift (.swift), Vue (.vue), Svelte (.svelte), PHP (.php), Scala (.scala), Zig (.zig), Lua (.lua), SQL (.sql), and Terraform/OpenTofu (.tf)..next,.nuxt,out,target,.dart_tool, andPodstoskipDirs..env.example,.env.sample,.env.template,.env.dist,.env.test) to prevent false-positive sandbox recommendations.prod/productiontokens (e.g.config/prod/app.json).2. Traits-Driven Model Selection & models.json Update
models.json: Upgraded Flash tier models to Gemini 3.8 Flash series.Claude Sonnet 4.6 (Thinking),GPT-OSS 120B (Medium)) inmain.gowith dynamic matching against modeltraits(instruction-following,quota-independent, etc.).3. Documentation & Testing
README.mdandREADME_ja.md.main_test.go(all tests passing).LESSONS.mdand updatedPLAN.md.