fix: support CREDIT_LIMIT quotas for Z.AI Coding Plan - #150
fix: support CREDIT_LIMIT quotas for Z.AI Coding Plan#150dylanzonghanyang-source wants to merge 7 commits into
Conversation
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
![opgg-ai-devops[bot]](https://avatars.githubusercontent.com/in/2680015?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR adds support for Z.AI Lite plans that return CREDIT_LIMIT windows, so users can finally see their 5-hour and 7-day quota data instead of getting a missing-usage error. The provider-side parsing and regression coverage are solid, and the existing schema path stays intact.
Blocking issue
The new weekly field stops at DetailedUsage/CLI/menu formatting. Existing status-bar selection, Recent Quota Change detection, and hasAnyValue handling still ignore it, so a real Lite account can show the wrong top-bar window, miss weekly-only changes, or hide its detail submenu. That needs to be wired through before merge; the inline comment has the exact consumers.
Smaller cleanup
There are a few easy surface-area cuts: remaining/number are decoded but unused, the numeric decoder is copied three times, and the credit-only predicate is repeated. These are non-blocking, but trimming them now keeps this provider from growing another mini schema layer.
Verification
- GitHub CI currently reports
Lint,Test,Build and Release, andCIas successful. - Local Swift/Xcode smoke tests could not run because this review environment is Linux and
xcodebuildis unavailable. - No new environment variables or dependencies were introduced.
df89742 to
0f2e2c9
Compare
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
There was a problem hiding this comment.
Summary
The Lite-tier CREDIT_LIMIT support is a solid direction. The legacy schema stays intact, the provider tests exercise the real async fetch path, and the new weekly fields make it through Codable, JSON, and the detail submenu. Nice work on the regression coverage here.
Blocking issue
The weekly value still stops short of every quota-row consumer. It is now eligible for status-bar selection, but the existing Z.AI top-level row paths still build their usedPercents arrays from only token and MCP values. A Lite account can therefore show the weekly window in one UI path but omit it from the provider row. The inline comment points at the new weekly candidate and both stale consumers; wire weeklyUsagePercent into those arrays and add a regression assertion for the top-level row.
Smaller cleanup
There is also some easy surface-area reduction: the new tests repeat the same envelope-decoding setup, and test-only visibility was added to production models and status-bar helpers. Keeping those models/helpers private and testing through the existing behavior path would avoid widening APIs just for tests.
Verification
- GitHub CI for this commit:
Lint,CI,Build and Release, andTestall passed. - Local Swift/Xcode smoke tests were unavailable because this review runner is Linux and has no
xcodebuild. - No new environment variables or dependencies were introduced.
Repo-wide audit
5건 — 펼쳐서 보기
- delete: Remove tracked generated artifacts that are not part of the application and only add stale backup/profile noise to the repository. [CopilotMonitor/CopilotMonitor.xcodeproj/project.pbxproj.bak, default.profraw]
- delete: Remove the unused MenuBarExtraAccess dependency and bridge state; no source imports or calls the package, while the app creates and manages its status item directly through AppKit. [CopilotMonitor/CopilotMonitor.xcodeproj/project.pbxproj, CopilotMonitor/CopilotMonitor.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved, CopilotMonitor/CopilotMonitor/App/ModernApp.swift, CopilotMonitor/CopilotMonitor/App/StatusBarController.swift]
- yagni: Collapse the separate ProviderManager and CLIProviderManager orchestration layers; both duplicate provider registration, parallel task-group fetching, timeout handling, and logging while differing mainly in presentation and error collection. [CopilotMonitor/CopilotMonitor/Services/ProviderManager.swift, CopilotMonitor/CLI/CLIProviderManager.swift]
- native: Use ArgumentParser's asynchronous command support instead of wrapping async work in DispatchSemaphore and nonisolated shared variables, which adds a manual concurrency bridge to the CLI. [CopilotMonitor/CLI/main.swift]
- delete: Remove the duplicated file-based debug appenders that write temporary provider_debug.log output alongside structured Logger calls; this is development instrumentation left in two production source files. [CopilotMonitor/CopilotMonitor/Services/ProviderManager.swift, CopilotMonitor/CopilotMonitor/App/StatusBarController.swift]
직전 라운드 미해결 (이번 라운드 미재론)
CopilotMonitor/CopilotMonitor/Providers/ZaiCodingPlanProvider.swift— schema predicate: selection logic can diverge (1라운드째)CopilotMonitor/CopilotMonitor/Providers/ZaiCodingPlanProvider.swift— numeric decoding: copies can drift (1라운드째)CopilotMonitor/CopilotMonitor/Providers/ZaiCodingPlanProvider.swift— unused quota fields: extra model surface has no consumer (1라운드째)
| case .zaiCodingPlan: | ||
| add(details?.mcpUsagePercent, priority: .monthly) | ||
| add(details?.tokenUsagePercent, priority: .hourly) | ||
| add(details?.weeklyUsagePercent, priority: .weekly) |
There was a problem hiding this comment.
[2라운드째 미합의]
This adds the weekly window to status-bar candidate selection, but the top-level Z.AI quota row still formats usedPercents from only token and MCP values at the two interaction sites below. For a Lite CREDIT_LIMIT account, the status bar can choose weekly while the provider row omits it. Add weeklyUsagePercent to both arrays (and a top-level-row regression assertion) so every display path agrees.
| } | ||
|
|
||
| private struct ZaiQuotaLimitItem: Decodable { | ||
| struct ZaiQuotaLimitItem: Decodable { |
There was a problem hiding this comment.
ZaiQuotaLimitItem is made module-visible only so the tests can decode the implementation model directly. Keep the response models private and exercise this through the injected fetch() path instead; that removes production API surface without losing behavioral coverage.
| // MARK: - Decoding | ||
|
|
||
| func testCreditLimitItemsDecode() throws { | ||
| struct Envelope: Decodable { |
There was a problem hiding this comment.
These three new decoding tests repeat the same local Envelope and JSONDecoder setup. Extract a small private helper that accepts the JSON string and returns the decoded limits, then keep each test focused on its assertion; otherwise a response-wrapper change has three maintenance sites.
| } | ||
|
|
||
| private func usagePercentCandidates( | ||
| static func usagePercentCandidates( |
There was a problem hiding this comment.
These helpers and their candidate types are made module-visible solely for direct tests. Keep the production helpers private and assert the existing controller behavior, or expose one narrowly scoped seam; exposing both algorithms makes future refactors preserve a test-only API.
Support the newer CREDIT_LIMIT quota schema returned by Z.AI / BigModel Coding Plan (lite tier) while preserving the existing TOKENS_LIMIT and TIME_LIMIT formats. - ZaiQuotaLimitItem: decode usage/unit; resolvedTotal falls back to usage when total is absent; computedPercentage prefers the API percentage field, else currentValue/resolvedTotal. remaining/number removed (no production consumer); server fixtures keep them to prove unknown fields do not break decoding - CREDIT_LIMIT-only responses keep BOTH rolling windows by unit (isCreditOnlySchema predicate named once): unit=3 -> 5-hour session quota (token usage fields), unit=6 -> 7-day weekly quota (new weeklyUsage* details). A single-window response maps only to its own window. Semantics verified against docs.z.ai FAQ, the Lite plan page, and third-party parsers (ClaudeBar ZaiUsageProbe, token-monitor) - Standard TOKENS_LIMIT/TIME_LIMIT behavior unchanged and wins over credit items in mixed responses - Status bar propagation: weeklyUsagePercent joins usagePercentCandidates with .weekly priority, usedPercentsForChangeDetection, and DetailedUsage.hasAnyValue; usagePercentCandidates and usedPercentsForChangeDetection made static (pure parameter functions) for direct testing - Menu: Z.AI submenu gains a Weekly (7d) window row - CLI: table shows session+weekly percentages; JSON adds weeklyUsagePercent/Used/Total/ResetsAt - Tests: provider-level fetch regressions (both windows, single window, standard schema unchanged, mixed schema), status-bar candidate priority, change detection, hasAnyValue, CLI formatters - register ZaiCodingPlanProviderTests in the Xcode project (previously present on disk but not compiled)
0f2e2c9 to
5d16c14
Compare
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
There was a problem hiding this comment.
Summary
This is a real fix: Z.AI Lite CREDIT_LIMIT responses now surface both the 5-hour and weekly quotas instead of failing with missing usage. The propagation through DetailedUsage, status-bar selection/change detection, the detail menu, and CLI formatters is solid, and the async regression coverage is honestly pretty good.
There are no correctness or security blockers left. I left six small, actionable maintenance comments around test-only API exposure, duplicated decoding/fixtures, and duplicated quota-row assembly. The whole-repo audit was completed by the workflow, and its audit details will be appended outside this payload.
CI is green (Lint, CI, Build and Release, and Test). Local Swift/Xcode smoke tests were unavailable because this Linux runner has no xcodebuild; no new dependencies or environment variables were introduced.
Verdict
The concrete functional benefit is that Z.AI Lite users can now see accurate 5-hour and weekly quota windows across the app instead of getting a missing-usage error. Approving this round; the remaining comments are non-blocking cleanup.
Repo-wide audit
5건 — 펼쳐서 보기
- delete: OpenCodeProvider is compiled but never instantiated or referenced by the application, leaving an entire provider implementation as dead maintenance surface alongside the active OpenCode Zen and OpenCode Go providers. [CopilotMonitor/CopilotMonitor/Providers/OpenCodeProvider.swift]
- delete: The MenuItemBuilder DSL is production-dead: no application code calls it, so the result-builder implementation and dedicated tests add maintenance surface without behavior. [CopilotMonitor/CopilotMonitor/Helpers/MenuResultBuilder.swift, CopilotMonitor/CopilotMonitorTests/MenuResultBuilderTests.swift, CopilotMonitor/CopilotMonitorTests/DependencyTests.swift]
- delete: ProviderManager.fetchAllResults is an unreferenced legacy wrapper around fetchAll, leaving a compatibility API with no consumer in the repository. [CopilotMonitor/CopilotMonitor/Services/ProviderManager.swift]
- shrink: The OpenCode binary discovery function is duplicated across two shell entry points, so the same lookup policy requires parallel edits and dozens of repeated lines. [scripts/query-opencode.sh, scripts/query-opencode-history.sh]
- delete: ProviderManager writes fetch events to /tmp/provider_debug.log while the same component already emits structured Logger records, duplicating logging behavior and retaining ad-hoc file I/O in the app path. [CopilotMonitor/CopilotMonitor/Services/ProviderManager.swift, CopilotMonitor/CopilotMonitor/App/StatusBarController.swift]
직전 라운드 미해결 (이번 라운드 미재론)
CopilotMonitor/CopilotMonitor/App/StatusBarController.swift— weekly window diverges: quota row omits Lite usage (2라운드째)
| } | ||
|
|
||
| private struct ZaiQuotaLimitResponse: Decodable { | ||
| struct ZaiQuotaLimitResponse: Decodable { |
There was a problem hiding this comment.
[2라운드째 미합의]
ZaiQuotaLimitResponse and ZaiQuotaLimitItem are internal only so the tests can decode them directly, which widens the production API for a test seam. Keep both DTOs private and exercise decoding through the injected fetch() path; the end-to-end tests already cover that behavior.
| /// row shows every active window side by side, so the Lite weekly window | ||
| /// must be included here too — omitting it makes the row diverge from the | ||
| /// usage windows (5h session, weekly, MCP monthly). | ||
| static func zaiCodingPlanTopLevelPercents(details: DetailedUsage?) -> [Double] { |
There was a problem hiding this comment.
[2라운드째 미합의]
This new helper and the candidate types are internal solely so tests can call implementation algorithms directly. Keep the production helpers private and assert the existing menu/status-bar behavior through one narrowly scoped seam instead, so future refactors do not preserve a test-only API.
| currentValue = Self.decodeInt(container, forKey: .currentValue) | ||
| total = Self.decodeInt(container, forKey: .total) | ||
| nextResetTime = Self.decodeInt64(container, forKey: .nextResetTime) | ||
| usage = Self.decodeInt(container, forKey: .usage) |
There was a problem hiding this comment.
[2라운드째 미합의]
The quota, model-usage, and tool-usage DTOs each copy the same Int/Double/String fallback decoder. Move that fallback into one private provider-level helper or decoding extension and reuse it from all three DTOs, otherwise schema fixes can drift between endpoints.
|
|
||
| // MARK: - Decoding | ||
|
|
||
| func testCreditLimitItemsDecode() throws { |
There was a problem hiding this comment.
[2라운드째 미합의]
These three tests recreate the same envelope decode and limits extraction, so a response-wrapper change has three maintenance sites. Add one private test helper that decodes creditOnlyJSON and returns the limits, then keep each test focused on its assertion.
| } | ||
| // MARK: - Z.AI CREDIT_LIMIT (lite tier) formatter tests | ||
|
|
||
| private func zaiCreditOnlyResult() -> ProviderResult { |
There was a problem hiding this comment.
The formatter test hardcodes the same Lite window values independently from the provider test fixture, so the two suites can stop representing the same API response without either failing for the intended reason. Share a small test fixture/constants source, or derive this result from the same decoded Lite payload.
| usedPercents = percents.isEmpty ? [account.usage.usagePercentage] : percents | ||
| } else if identifier == .zaiCodingPlan { | ||
| let percents = [account.details?.tokenUsagePercent, account.details?.mcpUsagePercent].compactMap { $0 } | ||
| let percents = Self.zaiCodingPlanTopLevelPercents(details: account.details) |
There was a problem hiding this comment.
The account-row and single-result branches repeat the full provider-specific percentage selection, so adding a window or correcting ordering requires two edits. Extract one helper that accepts DetailedUsage? and the fallback percentage, then use it from both branches.
❌ AI Code Review CancelledNewer commit detected or workflow was manually cancelled. 📋 View Logs | 🤖 Model: |
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
There was a problem hiding this comment.
Summary
This is a real fix: Z.AI Lite CREDIT_LIMIT responses now surface both the 5-hour and weekly quota windows instead of dropping the second window. The provider parsing and async regression coverage are solid, and the legacy schema path stays intact. Nice work on the actual product behavior here.
What blocks merge
The new testZaiTopLevelRowsRenderAllActiveWindows test does not compile: it dereferences optional NSMenuItem.attributedTitle with .map(\\.attributedTitle.string). The CI Test job catches this before any tests run. Fix the unwrap first; the inline comment points at the exact line.
The changed-file SwiftLint job also fails on a double blank line in the new test file (vertical_whitespace). Remove the extra empty line before rerunning CI.
Smaller cleanup
I left a few non-blocking maintenance comments around test-only production visibility, duplicated numeric decoding and fixtures, repeated quota-row assembly, and the reflection-based menu test seam. These are easy surface-area cuts, not correctness blockers.
Verification
- CI:
CIandBuild and Releasepassed;Testfailed at compilation;Lintfailed onvertical_whitespace. - Local Swift/Xcode smoke tests were unavailable because this review runner is Linux and has no
xcodebuild. - No new dependencies or environment variables were introduced.
The concrete functional benefit is that Z.AI Lite users can now see accurate 5-hour and weekly quota windows across the app instead of getting missing usage data. Requesting changes only for the compile/lint failures above; the remaining comments are non-blocking.
Repo-wide audit
6건 — 펼쳐서 보기
- delete: OpenCodeProvider is compiled but never instantiated or referenced by the application, leaving an entire provider implementation as dead maintenance surface beside the active OpenCode Zen and OpenCode Go providers. [CopilotMonitor/CopilotMonitor/Providers/OpenCodeProvider.swift]
- delete: The MenuItemBuilder DSL is production-dead because no application code calls it, so the result-builder implementation and dedicated tests add maintenance surface without behavior. [CopilotMonitor/CopilotMonitor/Helpers/MenuResultBuilder.swift, CopilotMonitor/CopilotMonitorTests/MenuResultBuilderTests.swift, CopilotMonitor/CopilotMonitorTests/DependencyTests.swift]
- delete: The SwiftUI status-bar shell is disabled and unreferenced while the AppKit status-bar controller is the active path, leaving duplicate view models, icon rendering, and an empty MenuBarExtra scene to maintain. [CopilotMonitor/CopilotMonitor/App/ModernApp.swift, CopilotMonitor/CopilotMonitor/Views/SwiftUI/ModernStatusBarIconView.swift]
- delete: ProviderManager.fetchAllResults is an unreferenced legacy wrapper around fetchAll, leaving a compatibility API with no consumer in the repository. [CopilotMonitor/CopilotMonitor/Services/ProviderManager.swift]
- shrink: The OpenCode binary discovery function is duplicated across two shell entry points, so the same lookup policy requires parallel edits and repeated lines. [scripts/query-opencode.sh, scripts/query-opencode-history.sh]
- delete: ProviderManager writes fetch events to /tmp/provider_debug.log while the same component already emits structured Logger records, duplicating logging behavior and retaining ad-hoc file I/O in the app path. [CopilotMonitor/CopilotMonitor/Services/ProviderManager.swift]
| total = Self.decodeInt(container, forKey: .total) | ||
| nextResetTime = Self.decodeInt64(container, forKey: .nextResetTime) | ||
| usage = Self.decodeInt(container, forKey: .usage) | ||
| unit = Self.decodeInt(container, forKey: .unit) |
There was a problem hiding this comment.
[3라운드째 미합의]
The quota, model-usage, and tool-usage DTOs each copy the same Int/Double/String fallback decoder. Extract one private generic helper parameterized by Key: CodingKey so schema fixes cannot drift between endpoints.
| return XCTFail("StatusBarController did not build its main menu") | ||
| } | ||
| let rows = menu.items | ||
| .map(\.attributedTitle.string) |
There was a problem hiding this comment.
attributedTitle is optional, so this key-path cannot compile in the test target. Unwrap it before mapping, for example with compactMap { $0.attributedTitle?.string }, then rerun the macOS test target.
| XCTAssertTrue(percents.contains(2), "weeklyUsagePercent missing from change detection: \(percents)") | ||
| } | ||
|
|
||
| /// The real demo/menu build path must render every active Z.AI window on |
There was a problem hiding this comment.
This recursive Mirror walk exists only to recover one private menu for a rendering assertion. Test the menu builder directly or expose one minimal internal seam, then delete the reflection and unwrap helpers; that is less brittle and removes roughly 20 lines.
| } | ||
|
|
||
| // === Weekly Usage (CREDIT_LIMIT unit=6, lite tier) === | ||
| if let weeklyUsage = details.weeklyUsagePercent { |
There was a problem hiding this comment.
The token, MCP, and weekly branches repeat the same usage-window plus optional limit-row sequence. Consolidate that into a small local helper accepting the label, values, reset date, and window style so future Z.AI windows do not create another drift point.
| // MARK: - Decoding | ||
|
|
||
| func testCreditLimitItemsDecode() throws { | ||
| struct Envelope: Decodable { |
There was a problem hiding this comment.
[3라운드째 미합의]
These three decoding tests repeat the same envelope decode and limits extraction. Add one private helper that accepts a JSON string and returns the limits, so a response-wrapper change has one maintenance site instead of three.
| } | ||
|
|
||
| private struct ZaiQuotaLimitResponse: Decodable { | ||
| struct ZaiQuotaLimitResponse: Decodable { |
There was a problem hiding this comment.
[3라운드째 미합의]
The response DTOs are made internal only so tests can decode them directly, coupling production visibility to the test structure. Keep both DTOs private and exercise decoding through the injected fetch() path instead.
| } | ||
| // MARK: - Z.AI CREDIT_LIMIT (lite tier) formatter tests | ||
|
|
||
| private func zaiCreditOnlyResult() -> ProviderResult { |
There was a problem hiding this comment.
[2라운드째 미합의]
This formatter suite hardcodes the same Lite window values independently from the provider fixture. Share a small fixture/constants source, or derive this result from the decoded Lite payload, so both suites cannot silently represent different API responses.
| usedPercents = percents.isEmpty ? [account.usage.usagePercentage] : percents | ||
| } else if identifier == .zaiCodingPlan { | ||
| let percents = [account.details?.tokenUsagePercent, account.details?.mcpUsagePercent].compactMap { $0 } | ||
| let percents = Self.zaiCodingPlanTopLevelPercents(details: account.details) |
There was a problem hiding this comment.
[2라운드째 미합의]
The account-row and single-result branches still duplicate the provider-specific percentage selection. Extract one helper accepting DetailedUsage? plus the fallback percentage and use it from both branches, so adding the next window needs one edit.
| } | ||
|
|
||
| private enum UsageDisplayWindowPriority: Int, CaseIterable { | ||
| enum UsageDisplayWindowPriority: Int, CaseIterable { |
There was a problem hiding this comment.
[3라운드째 미합의]
These controller algorithms and candidate types are made module-visible solely for direct tests. Keep them private and test the existing behavior path, or expose one narrowly scoped seam instead of widening several production APIs.
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
There was a problem hiding this comment.
Summary
This is a real fix: Z.AI Lite CREDIT_LIMIT responses now surface the 5-hour and weekly quota windows across the provider, status bar, detail menu, and CLI output. The async regression coverage is genuinely solid, and the legacy schema stays intact. Nice work on the product behavior here.
I found one forward-compatibility concern in the credit-window mapping plus a few non-blocking maintenance issues around test seams and duplicated schema/rendering lists. None of the repository's known-critical security patterns matched the manifests, and this PR adds no dependencies or environment variables.
Verification
- GitHub CI:
Lint,CI, andBuild and Releaseare green;Testwas still in progress when fetched. - Local macOS/Xcode smoke tests are unavailable on this Linux runner. The repository has no TypeScript project, so the fallback
tscsmoke signal is not meaningful;npm testis not defined. - No new environment-variable or secret usage appeared in the diff.
Verdict
The concrete benefit is that Z.AI Lite users can now see accurate 5-hour and weekly quota windows instead of missing usage data. Approving this round; the remaining comments are actionable cleanup and forward-compatibility hardening, not a verified caller-visible break today.
Repo-wide audit
4건 — 펼쳐서 보기
- delete: Remove the disabled SwiftUI menu shell, unused status-bar bridge, and abandoned SwiftUI view-model/icon layer; the app uses the native AppKit status item directly, so this parallel architecture and its MenuBarExtraAccess dependency add maintenance surface without a live caller. [CopilotMonitor/CopilotMonitor/App/ModernApp.swift, CopilotMonitor/CopilotMonitor/App/StatusBarController.swift, CopilotMonitor/CopilotMonitor/ViewModels/ProviderViewModel.swift, CopilotMonitor/CopilotMonitor/Views/SwiftUI/ModernStatusBarIconView.swift, CopilotMonitor/CopilotMonitor.xcodeproj/project.pbxproj]
- delete: Remove OpenCodeProvider; it is not registered by the application or CLI and has no callers, leaving an entire provider implementation and response model dead in the shipped target. [CopilotMonitor/CopilotMonitor/Providers/OpenCodeProvider.swift, CopilotMonitor/CopilotMonitor/Services/ProviderManager.swift, CopilotMonitor/CLI/CLIProviderManager.swift]
- yagni: Collapse the CLI provider registry into the actual provider array instead of maintaining a second manually duplicated list; the current two sources of truth can drift and make the list command report providers the manager does not fetch. [CopilotMonitor/CLI/CLIProviderManager.swift]
- delete: Remove the unused fetchAllResults compatibility wrapper; repository call sites use fetchAll directly, so this dead API preserves an unnecessary legacy surface. [CopilotMonitor/CopilotMonitor/Services/ProviderManager.swift]
교착 (사람 판단 필요)
같은 지적이 여러 라운드 반복됐으나 합의에 이르지 못했습니다. 인라인 반복을 멈추고 여기 남깁니다 — 판단이 서면 다시 멘션해 주세요.
CopilotMonitor/CopilotMonitorTests/ZaiCodingPlanProviderTests.swift— repeated envelope setup: fixture changes can drift (4라운드째)CopilotMonitor/CopilotMonitor/Providers/ZaiCodingPlanProvider.swift— duplicate numeric decoding: schema fixes can drift (4라운드째)CopilotMonitor/CopilotMonitor/App/StatusBarController.swift— test-only visibility: controller internals become API (4라운드째)CopilotMonitor/CopilotMonitor/Providers/ZaiCodingPlanProvider.swift— test-only DTO visibility: production surface expands (4라운드째)
직전 라운드 미해결 (이번 라운드 미재론)
CopilotMonitor/CopilotMonitor/App/StatusBarController.swift— Duplicated quota-row assembly: account paths can drift (2라운드째)CopilotMonitor/CopilotMonitorTests/ZaiCodingPlanProviderTests.swift— Optional title access: test target cannot compile (1라운드째)
| } | ||
| // MARK: - Z.AI CREDIT_LIMIT (lite tier) formatter tests | ||
|
|
||
| private func zaiCreditOnlyResult() -> ProviderResult { |
There was a problem hiding this comment.
[3라운드째 미합의]
This formatter suite hardcodes the same Lite window values as creditOnlyJSON in the provider tests. Share a small fixture/constants source, or derive this result from the decoded payload, so the provider and formatter suites cannot silently describe different API responses.
| /// a production-only test accessor to StatusBarController. | ||
| @MainActor | ||
| private func menu(from controller: StatusBarController) -> NSMenu? { | ||
| guard let value = Mirror(reflecting: controller).children |
There was a problem hiding this comment.
[2라운드째 미합의]
This Mirror walk reaches the private menu by storage shape, so a harmless property rename or layout change breaks the test even when menu behavior is unchanged. Test the rendered result through a narrow internal seam or extract row-building into a directly testable value-producing function instead of reflecting over object storage.
| /// Window unit used to distinguish the plan's rolling windows: | ||
| /// unit=3 (hours) -> 5-hour session quota, unit=6 (weeks) -> 7-day weekly quota. | ||
| /// See docs.z.ai FAQ and third-party parsers (ClaudeBar ZaiUsageProbe, token-monitor). | ||
| let unit: Int? |
There was a problem hiding this comment.
unit is enough for the two known Lite windows today, but this decoder drops the response's number/remaining metadata and then hard-codes every unit == 3 item into the Tokens (5h) slot. If Z.AI returns another hour-based credit window or changes the reported duration, the app will show the wrong label and treat currentValue as the right total without checking the server's remaining balance. Decode the duration/capacity metadata and add a fixture that varies it; keep a narrowly documented fallback for the current Lite shape.
| /// row shows every active window side by side, so the Lite weekly window | ||
| /// must be included here too — omitting it makes the row diverge from the | ||
| /// usage windows (5h session, weekly, MCP monthly). | ||
| private static func zaiCodingPlanTopLevelPercents(details: DetailedUsage?) -> [Double] { |
There was a problem hiding this comment.
The top-level row, candidate selection, and change-detection paths each enumerate the Z.AI windows separately. Define one shared window descriptor with its field and display priority, then derive these lists from it while preserving each path's ordering; otherwise the next window addition needs synchronized edits and can silently drift.
| } | ||
|
|
||
| // === Weekly Usage (CREDIT_LIMIT unit=6, lite tier) === | ||
| if let weeklyUsage = details.weeklyUsagePercent { |
There was a problem hiding this comment.
[2라운드째 미합의]
The new Z.AI weekly branch repeats the optional-window/separator/row sequence already hand-written for several providers. Build a small window-descriptor list and one shared add-window routine for separator handling, leaving only provider-specific labels and fields in data; that keeps future window rows from diverging.
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
There was a problem hiding this comment.
Summary
This is a real fix: Z.AI Lite CREDIT_LIMIT responses now surface the 5-hour and weekly quota windows across the provider, status bar, detail menu, and CLI output. The async regression coverage is genuinely solid, and the legacy schema stays intact. Nice work on the product behavior here.
I found one forward-compatibility concern in the credit-window mapping plus a few non-blocking maintenance comments around test seams and duplicated schema/rendering lists. No known-critical security exposure matched the repository manifests, and this PR adds no dependencies or environment variables.
The workflow's repo-wide audit will be appended outside this payload.
Verification
- GitHub CI is green:
Lint,Test,Build and Release, andCIall passed. - Local Swift/Xcode smoke tests were unavailable because this Linux runner has no
xcodebuild. - No new environment-variable or secret usage appeared in the diff.
Verdict
The concrete benefit is that Z.AI Lite users can now see accurate 5-hour and weekly quota windows instead of missing usage data. Approving this round; the remaining comments are actionable cleanup and forward-compatibility hardening, not a verified caller-visible break today.
Repo-wide audit
6건 — 펼쳐서 보기
- yagni: The app and CLI each implement a separate provider-fetch coordinator with duplicated task-group, timeout, logging, and provider registration logic, so one shared coordinator can remove a second orchestration layer. [CopilotMonitor/CopilotMonitor/Services/ProviderManager.swift, CopilotMonitor/CLI/CLIProviderManager.swift]
- shrink: Five provider test suites carry near-identical MockURLProtocol and ephemeral URLSession setup instead of sharing one test support helper, multiplying maintenance for the same harness. [CopilotMonitor/CopilotMonitorTests/MiniMaxProviderTests.swift, CopilotMonitor/CopilotMonitorTests/NanoGptProviderTests.swift, CopilotMonitor/CopilotMonitorTests/GeminiCLIProviderTests.swift, CopilotMonitor/CopilotMonitorTests/ZaiCodingPlanProviderTests.swift, CopilotMonitor/CopilotMonitorTests/SyntheticProviderTests.swift]
- shrink: The two OpenCode shell queries duplicate the complete binary-discovery function and its error handling, so changes to installation-path support must be copied between scripts. [scripts/query-opencode.sh, scripts/query-opencode-history.sh]
- shrink: ProviderIdentifier repeats the same provider metadata as three large switch statements for displayName, shortDisplayName, and iconName; one metadata table would remove parallel case maintenance. [CopilotMonitor/CopilotMonitor/Models/ProviderProtocol.swift]
- shrink: StatusCommand and ProviderCommand manually bridge async work with DispatchSemaphore and nonisolated shared variables, duplicating substantial synchronization scaffolding that AsyncParsableCommand already provides. [CopilotMonitor/CLI/main.swift]
- delete: ProviderCommand checks the lowercased raw value once through ProviderIdentifier(rawValue:) and then repeats the same exact raw-value lookup in a loop, making the second search dead redundancy. [CopilotMonitor/CLI/main.swift]
교착 (사람 판단 필요)
같은 지적이 여러 라운드 반복됐으나 합의에 이르지 못했습니다. 인라인 반복을 멈추고 여기 남깁니다 — 판단이 서면 다시 멘션해 주세요.
CopilotMonitor/CopilotMonitorTests/CLIFormatterTests.swift— Lite fixture: provider and formatter tests can drift (4라운드째)
| return URLSession(configuration: configuration) | ||
| } | ||
|
|
||
| /// Read the menu produced by the real controller build path without adding |
There was a problem hiding this comment.
[3라운드째 미합의]
This test recursively reflects over private storage just to recover one menu, so a harmless property rename or storage-layout change breaks behavior coverage. Expose one narrowly scoped internal test seam or extract row-building into a directly testable value-producing function, then delete menu(from:) and unwrapMenu(_:).
| submenu.addItem(item) | ||
| } | ||
|
|
||
| // === Weekly Usage (CREDIT_LIMIT unit=6, lite tier) === |
There was a problem hiding this comment.
[3라운드째 미합의]
The Z.AI token, MCP, and weekly branches repeat the same usage-window and limit-row construction used by sibling provider branches. Move the optional-window rendering into a small helper driven by window definitions, keeping only provider-specific labels and fields in data, so separators and reset handling do not drift across synchronized edits.
| /// row shows every active window side by side, so the Lite weekly window | ||
| /// must be included here too — omitting it makes the row diverge from the | ||
| /// usage windows (5h session, weekly, MCP monthly). | ||
| private static func zaiCodingPlanTopLevelPercents(details: DetailedUsage?) -> [Double] { |
There was a problem hiding this comment.
[2라운드째 미합의]
Z.AI windows are still enumerated separately for the top-level row, status-bar candidate selection, and change detection. Define one shared window descriptor with its field and display priority, then derive each ordered list from it; otherwise adding the next window needs synchronized edits and can silently make one path disagree with the others.
| // parsers). Keep BOTH windows; the weekly cap is the one users care about. | ||
| let creditLimits = limits.filter { $0.type.uppercased() == "CREDIT_LIMIT" } | ||
| let isCreditOnlySchema = tokenLimit == nil && mcpLimit == nil | ||
| let creditSessionLimit = isCreditOnlySchema ? creditLimits.first { $0.unit == 3 } : nil |
There was a problem hiding this comment.
[2라운드째 미합의]
unit is currently the only discriminator here, so any additional CREDIT_LIMIT entry with unit 3 is silently labeled as the 5-hour session window and its usage is treated as that window's capacity. The API also provides number and remaining, and this parser drops them. Decode and validate the server's duration/capacity metadata (with a documented fallback for the currently observed unit values) and add a fixture that varies it; otherwise a future response can render the wrong window and quota total.
| let currentValue: Int? | ||
| let total: Int? | ||
| let nextResetTime: Int64? | ||
| /// CREDIT_LIMIT items (lite tier) report capacity as `usage` and leftover as `remaining` |
There was a problem hiding this comment.
This comment says CREDIT_LIMIT replaces both total and currentValue, but the decoder still uses currentValue as the consumed amount and only falls back from total to usage for capacity. Rewrite the comment to match that contract so a future schema cleanup does not remove the required used-value field.
Combine the green Z.AI CREDIT_LIMIT implementation from PR opgginc#150 with the approved DeepSeek balance implementation from PR opgginc#151. Preserve both provider test registrations and formatter coverage.
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
There was a problem hiding this comment.
Summary
This is a real fix: Z.AI Lite CREDIT_LIMIT responses now surface both the 5-hour and weekly quota windows across the provider, status bar, detail menu, and CLI output. The async regression coverage is solid, and the legacy schema path stays intact. Nice work on the actual product behavior here.
What blocks merge
The new weekly detail row drops its reset-time UI. The branch passes weeklyUsageReset into createUsageWindowRow, but not the seven-day duration that the shared helper needs to build paceInfo; the helper returns before adding the Resets: item. Lite users can therefore see the weekly percentage and totals but not when that quota resets. The inline comment has the exact fix and a regression-test direction.
Smaller cleanup
I also left a few non-blocking maintenance comments around the test-only response surface, reflection-based menu access, duplicated test harness, duplicated window lists, and the credit-window contract. These are real surface-area cuts, not blockers by themselves.
Verification
- GitHub CI is green:
Lint,Test,Build and Release, andCIall passed. - Local Swift/Xcode smoke tests were unavailable because this Linux runner has no
xcodebuild; the repo has no lockfiles for a safe dependency install fallback. - No new dependencies, environment variables, or secret references appeared in the diff.
- The existing manifests do not contain the org's known-critical React2Shell packages.
- The workflow completed the repo-wide audit; its details are appended outside this payload.
Verdict
The concrete benefit is that Z.AI Lite users can now see accurate 5-hour and weekly quota windows instead of missing usage data. Requesting changes because the new weekly window currently hides a caller-visible reset time; the remaining comments are non-blocking cleanup.
Repo-wide audit
4건 — 펼쳐서 보기
- delete: Repeated ad-hoc debugLog plumbing appends to the same /tmp/provider_debug.log despite structured os.Logger already being present, adding production file I/O and duplicated call sites with no durable product behavior. [CopilotMonitor/CopilotMonitor/App/StatusBarController.swift, CopilotMonitor/CopilotMonitor/Services/ProviderManager.swift, CopilotMonitor/CopilotMonitor/Services/BrowserCookieService.swift, CopilotMonitor/CopilotMonitor/Services/TokenManager.swift]
- shrink: The identical OpenCode binary-discovery function is copied into both shell entry points, so path strategy changes must be maintained twice. [scripts/query-opencode.sh, scripts/query-opencode-history.sh]
- shrink: Five test classes carry the same MockURLProtocol implementation, session factory, and teardown boilerplate, creating a large duplicated test harness surface. [CopilotMonitor/CopilotMonitorTests/MiniMaxProviderTests.swift, CopilotMonitor/CopilotMonitorTests/NanoGptProviderTests.swift, CopilotMonitor/CopilotMonitorTests/GeminiCLIProviderTests.swift, CopilotMonitor/CopilotMonitorTests/ZaiCodingPlanProviderTests.swift, CopilotMonitor/CopilotMonitorTests/SyntheticProviderTests.swift]
- delete: package.json exposes an unused npm lint wrapper and duplicate action-validator dependency while the documented Makefile already owns the lint-actions command and no repository workflow invokes the npm script. [package.json, Makefile]
교착 (사람 판단 필요)
같은 지적이 여러 라운드 반복됐으나 합의에 이르지 못했습니다. 인라인 반복을 멈추고 여기 남깁니다 — 판단이 서면 다시 멘션해 주세요.
CopilotMonitor/CopilotMonitorTests/ZaiCodingPlanProviderTests.swift— reflection test seam: storage changes break coverage (4라운드째)CopilotMonitor/CopilotMonitor/Helpers/ProviderMenuBuilder.swift— window rendering duplicated: quota rows can drift (4라운드째)
| /// is absent. `currentValue` remains the consumed amount, while | ||
| /// `remaining` is server-reported leftover metadata. | ||
| let usage: Int? | ||
| /// Optional duration metadata for identifying known CREDIT_LIMIT windows. |
There was a problem hiding this comment.
[2라운드째 미합의]
This comment says number is duration metadata, but the implementation also accepts a missing value and ignores contradictory values. That makes the schema contract pretty fuzzy. Document the exact unit/number matching plus the legacy nil-number fallback, or simplify the comment to the guarantee the code actually provides.
|
|
||
| // === Weekly Usage (CREDIT_LIMIT unit=6, lite tier) === | ||
| if let weeklyUsage = details.weeklyUsagePercent { | ||
| let items = createUsageWindowRow( |
There was a problem hiding this comment.
This weekly branch passes the reset date but not the window duration, so createUsageWindowRow reaches the paceInfo guard and returns before adding the Resets: item. Lite users get the percentage and totals but no reset time. Pass windowHours: 24 * 7 (or an equivalent weekly input) and add a regression assertion for the rendered reset row.
| let creditSessionLimit = isCreditOnlySchema | ||
| ? creditLimits.first { $0.unit == 3 && ($0.number == nil || $0.number == 5) } | ||
| : nil | ||
| let creditWeeklyLimit = isCreditOnlySchema |
There was a problem hiding this comment.
[3라운드째 미합의]
The mapping only recognizes the two hard-coded unit/number pairs, then treats missing duration metadata as compatible. A new hour/week duration can therefore disappear or be represented as the wrong window. Decode and validate the server's duration/capacity identity explicitly, and keep the nil-number fallback narrowly documented and covered by a legacy fixture.
| /// row shows every active window side by side, so the Lite weekly window | ||
| /// must be included here too — omitting it makes the row diverge from the | ||
| /// usage windows (5h session, weekly, MCP monthly). | ||
| private static func zaiCodingPlanTopLevelPercents(details: DetailedUsage?) -> [Double] { |
There was a problem hiding this comment.
[3라운드째 미합의]
The top-level row now has its own Z.AI window list, while candidate selection and change detection still enumerate the fields separately. Define one ordered window descriptor and derive these consumer lists from it; otherwise the next quota window needs synchronized edits and one path can silently disagree.
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
There was a problem hiding this comment.
Summary
This PR has a concrete benefit: Z.AI Lite/CREDIT_LIMIT accounts now expose both the 5-hour session quota and the rolling weekly quota across the provider model, menu, status bar, persistence, and formatters. The end-to-end coverage is solid, especially the real async fetch path and the duration-mismatch cases. Nice work on keeping the existing TOKENS_LIMIT/TIME_LIMIT behavior intact.
I left one small maintainability comment on the remaining duplicated Z.AI window lists. Nothing here looks like a correctness or security blocker, so this is an approval.
Checks
- CI: Lint, Build and Release, and the general CI workflow passed; the Test workflow is still in progress.
- Local smoke tests: not run. The prefetch found no lockfiles, and this Linux runner cannot execute the repo's authoritative macOS/Xcode build and XCTest path.
- No new dependencies or environment variables were introduced.
- The whole-repo audit completed separately; its findings are appended by the review workflow.
Repo-wide audit
5건 — 펼쳐서 보기
- delete: Remove committed generated artifacts that are not source or release inputs and only add repository noise. [default.profraw, scripts/__pycache__/browser_cookies.cpython-312.pyc]
- delete: Remove the per-component /tmp file debugLog implementations and their duplicate call sites; the app already has structured os.Logger logging, so these add a second unbounded logging system and several maintenance surfaces. [CopilotMonitor/CopilotMonitor/App/StatusBarController.swift, CopilotMonitor/CopilotMonitor/Services/ProviderManager.swift, CopilotMonitor/CopilotMonitor/Services/BrowserCookieService.swift, CopilotMonitor/CopilotMonitor/Providers/OpenCodeZenProvider.swift, CopilotMonitor/CopilotMonitor/Providers/CommandCodeProvider.swift, CopilotMonitor/CopilotMonitor/Providers/CursorProvider.swift, CopilotMonitor/CopilotMonitor/Providers/KiroProvider.swift]
- shrink: Consolidate the identical JSON-comment stripper, environment-value resolver, and configuration path discovery duplicated between the Brave Search and Tavily scripts instead of maintaining two copies of the same parser and search-path policy. [scripts/query-brave-search.sh, scripts/query-tavily-search.sh]
- shrink: Share the duplicated Codex JWT decoding and usage-formatting logic; the two scripts differ mainly in credential-file extraction, while their request, decoding, and jq presentation paths are repeated. [scripts/query-codex.sh, scripts/query-codex-native.sh]
- shrink: Collapse findProvider into one case-insensitive lookup pass; the normalized raw-value comparison makes the exact raw-value loop redundant, and the current implementation scans the same enum repeatedly before doing the display-name match. [CopilotMonitor/CLI/main.swift]
교착 (사람 판단 필요)
같은 지적이 여러 라운드 반복됐으나 합의에 이르지 못했습니다. 인라인 반복을 멈추고 여기 남깁니다 — 판단이 서면 다시 멘션해 주세요.
CopilotMonitor/CopilotMonitor/App/StatusBarController.swift— duplicated window lists: status-bar paths can drift (4라운드째)
직전 라운드 미해결 (이번 라운드 미재론)
CopilotMonitor/CopilotMonitor/Providers/ZaiCodingPlanProvider.swift— schema contract unclear: fallback behavior drifts (2라운드째)CopilotMonitor/CopilotMonitor/Helpers/ProviderMenuBuilder.swift— weekly reset input missing: reset row disappears (1라운드째)CopilotMonitor/CopilotMonitor/Providers/ZaiCodingPlanProvider.swift— credit-window mapping permissive: future windows mislabel (3라운드째)
Integrates the final fix/zai-credit-limit branch: CREDIT_LIMIT parsing, duration identity (3/5 session, 6/1 weekly, contradictory-number rejection, missing-number compatibility fallback), weekly DetailedUsage propagation, status-bar weekly candidate and change detection, top-level weekly row, weekly detail reset row (windowHours 24*7), and CLI table/JSON support.
1 participant
Problem
Z.AI / BigModel Coding Plan (China) has introduced a newer quota schema for
the Lite tier. The current provider only recognizes
TOKENS_LIMITandTIME_LIMITwindows, so Lite accounts receive a "Missing usage percentages"error and show nothing.
Current behavior
ZaiQuotaLimitItemonly decodespercentage / currentValue / total / nextResetTime. Lite responses carry neithertotalnorTOKENS_LIMIT/TIME_LIMITitems, so parsing fails.New response format
{ "data": { "limits": [ { "type": "CREDIT_LIMIT", "unit": 3, "number": 5, "usage": 2000, "currentValue": 27, "remaining": 1972, "percentage": 1, "nextResetTime": 1786717056698 }, { "type": "CREDIT_LIMIT", "unit": 6, "number": 1, "usage": 10000, "currentValue": 27, "remaining": 9972, "percentage": 1, "nextResetTime": 1787301777997 } ], "level": "lite" } }CREDIT_LIMITitems report capacity asusageand leftover asremaininginstead of
total. Two items are returned, one per rolling window:unit=3(hours) is the 5-hour session quota,unit=6(weeks) is the 7-dayweekly quota — verified against the docs.z.ai FAQ, the Lite plan page
("10,000 Credits / week"), and third-party parsers (ClaudeBar
ZaiUsageProbe, token-monitorzaiLimits).Implementation
ZaiQuotaLimitItemdecodesusage/remaining/unit/number;resolvedTotalfalls back tousagewhentotalis absent;computedPercentageprefers the API-providedpercentage, else derivescurrentValue / resolvedTotal.CREDIT_LIMIT-only responses keep both windows: the 5-hour sessionquota surfaces through the existing token-usage fields, and the weekly
quota surfaces through new
weeklyUsage*details fields (plus a"Weekly (7d)" row in the Z.AI submenu).
TOKENS_LIMIT/TIME_LIMITselection is unchanged and winsover credit items in mixed responses.
Backward compatibility
Existing plans keep the exact previous behavior; the weekly fields are
optional and only populated on the
CREDIT_LIMIT-only path.Tests
Provider-level regression tests run the real
fetch()pipeline with aninjected API key and mocked endpoints (no credential store / network):
CREDIT_LIMIT-only with both windows -> both windows present inProviderResult.details(session + weekly)CREDIT_LIMIT-only with a single window maps only to its own windowTOKENS_LIMIT/TIME_LIMITschema unchangedunit/number/usage/remaining,resolvedTotalfallback,percentage derivation
weeklyUsagePercent/Used/Total/ResetsAtZaiCodingPlanProviderTestsis registered in the Xcode project (it waspreviously present on disk but never compiled)
Runtime verification
Validated against a real Lite Coding Plan account. CLI smoke test on the
built artifact:
Both the 5-hour session window and the weekly window render correctly with
used/total and reset times. No account-specific data included.