fix(confgen): dedup refer load errors and locate them to referred target - #446
Open
Kybxd wants to merge 3 commits into
Open
fix(confgen): dedup refer load errors and locate them to referred target#446Kybxd wants to merge 3 commits into
Kybxd wants to merge 3 commits into
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #446 +/- ##
==========================================
+ Coverage 75.58% 75.79% +0.21%
==========================================
Files 88 88
Lines 9593 9615 +22
==========================================
+ Hits 7251 7288 +37
+ Misses 1760 1748 -12
+ Partials 582 579 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Kybxd
force-pushed
the
fix/refer-load-failure-dedup
branch
from
August 17, 2026 06:54
fe1aee8 to
ce9ae11
Compare
When a refer target (e.g. AssistSkillConf.ID) fails to load its value space -- for instance because the referred sheet has a broken row that makes RangeDataRows return an error via Ignored()/parseColumns -- every row on the referring side would re-invoke loadFunc and re-emit the same error, producing N duplicate errors for one root cause. - ReferredCache now tracks a 'failed' set alongside 'references'. The first loadFunc failure is still surfaced to the caller (so the error is not swallowed), but subsequent ExistsValue calls on the same refer short-circuit to (true, nil) instead of re-triggering the load. - AddFromTable wraps RangeDataRows errors with KeyReferBookName / KeyReferSheetName, so outer confgen wrappers (which set BookName / SheetName to the source sheet under generation) no longer obscure where the offending row actually lives. - xerrors adds KeyReferBookName / KeyReferSheetName; zh/en templates render the referred workbook/worksheet when present.
… runs Patch coverage was failing because the failed-refer short-circuit and ReferBookName wrapping had no tests. Reset the process-level cache at each GenAll/GenWorkbook so a prior failure cannot leak into the next run. Co-authored-by: Cursor <cursoragent@cursor.com>
Kybxd
force-pushed
the
fix/refer-load-failure-dedup
branch
from
August 17, 2026 06:55
ce9ae11 to
4abd45d
Compare
…ess global A package-level cache leaked failed loads and stale value spaces across Generate/Load calls. Keep one cache per Generator (and per loadOrigin) and pass it through SheetParserExtInfo into InReferredSpace. Co-authored-by: Cursor <cursoragent@cursor.com>
Kybxd
force-pushed
the
fix/refer-load-failure-dedup
branch
from
August 17, 2026 07:11
4abd45d to
63545b0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a refer target (e.g.
AssistSkillConf.ID) fails to load its value space -- for instance because the referred sheet has a broken row that makesRangeDataRowsreturn an error viaIgnored()/parseColumns/ etc. -- every row on the referring side would re-invokeloadFuncand re-emit the same error, producing N duplicate errors for one root cause.Additionally, those N errors were wrapped with the source sheet's
BookName/SheetName(the sheet under generation), which pointed users away from the sheet actually containing the offending row.Changes
ReferredCache: negative cachingfailed map[string]struct{}tracks refer expressions whose load has already failed.loadFunc(refer)error, the failure is still surfaced to the caller (the collector receives the real error once), and the refer is marked as failed.ExistsValuecalls for the same refer short-circuit to(true, nil)instead of re-triggering the load. This preserves error visibility while eliminating the N-way duplication.failedthe same way asreferences, so concurrent callers don't race into duplicate loads.AddFromTable: locate errors on the referred targetRangeDataRowserrors withKeyReferBookName/KeyReferSheetName.BookName/SheetNameto the source sheet under generation (which is what users need to find the refer expression in the schema), while the new keys pinpoint the referred workbook / worksheet that actually contains the offending row.Supporting bits
xerrors/desc.go: addKeyReferBookName/KeyReferSheetNameconstants and register them inkeys.i18ntemplates (zh.yaml/en.yaml): renderReferWorkbook/ReferWorksheet(引用工作簿 / 引用工作表) when present.Behavior after this PR
Given
AffixConfreferringAssistSkillConf.IDandAssistSkill.xlsxhaving a broken row (e.g. an#IGNOREcell that fails to parse as bool):AssistSkill.xlsxself-generation errorAffixConffirst refer check that triggers the loadBookName/SheetNamein the refer-side errorReferBookName/ReferSheetNamepointing atAssistSkill.xlsxVerification
go build ./...go test ./internal/confgen/... ./internal/confgen/fieldprop/...