This is a submission for DEV's Summer Bug Smash: Clear the Lineup powered by Sentry.
When the Summer Bug Smash challenge started, I gave myself one rule. Every fix has to be real, and every fix has to carry its own test that fails before the change and passes after it. I gave myself one day to see how far that rule could take me. It took me through three open source projects, three pull requests, and a lot of green check marks. Here is how it went, bug by bug.
And btw, my GitHub username is @aniruddhaadak80.
Enhanced Search Coverage for Lazy-Loaded Sheets
I recently addressed a significant issue where users searching for data in large, lazy-loaded spreadsheets received incorrect results because the Find dialog only scanned rows already visible in the grid. To fix this, I registered a wrapper find provider that extends the search process to include out-of-window matches by paging data directly from the underlying file. When a user selects a result that is not currently loaded, I implemented logic that automatically activates the sheet, loads the necessary range, and scrolls the view so that the grid displays the actual data instead of a blank region.
I ensured that this new implementation maintains consistency with existing system behaviors by mirroring established semantics for case sensitivity and formula lookups. I also added a status message to inform users when a scan hits performance limits, which prevents the confusion caused by silently skipped data. Because I utilized public extension points rather than patching internal code, this solution remains stable across different workbook switches and allows for seamless navigation between the native model and my extended search functionality.
fix(sheets): extend Ctrl+F beyond the loaded window on streamed workbooks
#131
-
What changed? On streamed (lazy-loaded) workbooks, the Find dialog now searches the whole sheet instead of only the rows already loaded into Univer's grid. A wrapper find provider is registered with
IFindReplaceService: the built-in sheets model keeps owning everything inside the loaded window (including its canvas highlights), while the wrapper extends each session with out-of-window matches paged from the underlying file viareadSheetRangeMapped— session journal edits included, coordinates already covered by the loaded window excluded. Focusing an out-of-window match activates its sheet, starts loading its range (ensureLazyRangeLoaded), scrolls to it, and selects it, so the grid shows real data instead of jumping to a blank region. Replace All reports out-of-window formula hits as failures instead of silently skipping them, consistent with the existing guard that blocks bulk replaces until the workbook is fully loaded. When a scan hits the budget or indexing lag, a status message says results may be incomplete (newappFindScanTruncatedstring, all 19 locales). -
Why is this change needed? The Find dialog iterates Univer's in-memory cell matrix, so on large lazy-loaded sheets every not-yet-streamed row was silently skipped: users searched for values that exist and got "no results", concluding the data does not exist. The AI-side
findInLazyWorkbookalready pages the underlying file for exactly this reason; this brings the UI dialog to parity using the same primitives (readSheetRangeMapped,ensureLazyRangeLoaded) and the same budgets (FILE_READ_BATCH_CELLS,MAX_SCAN_CELLS, now exported fromai/workbook-search.ts).
Implementation notes:
- Uses Univer's public extension point (
registerFindReplaceProvider); no prototype patching of Univer internals. The wrapper re-adopts whichever sheets provider is registered, so it survives workbook switches, and delegates unchanged for demo/fully-preloaded workbooks. - Matching mirrors the built-in semantics (substring vs whole-cell with space-only trimming, case sensitivity, formula-vs-value look-in); needles are lower-cased once up front like the built-in parser does. Formula hits are never value-replaced, mirroring
hitCell. - Out-of-window hits are deduped against the live inner list on every
getMatches(), so as regions stream in or evict, counts stay correct and navigation hands back to the native model once a jumped-to cell is materialized (the inner model re-runs on mutations). - New module
apps/sheets/src/renderer/lazy-find.ts(pure helpers exported for tests) wired from the App mount effect; two new direct deps already present transitively:@univerjs/find-replace,rxjs.
Known limitations (deliberate scope cuts, happy to iterate):
- Filtered-out rows are not excluded from out-of-window hits (the in-window scan still skips them natively).
- Extras refresh when a search starts; between searches only the inner list refreshes live.
- Replace on an unloaded plain-cell hit writes straight onto the cell; the edit journal replays it onto later viewport installs, so saves and AI reads stay correct.
Closes #113
- [x]
npm run format:check - [x]
npm run lint— scoped ESLint run over all changed files: 0 errors (the 3 pre-existingreact-hooks/exhaustive-depswarnings in App.tsx's untouched cleanup block remain warnings) - [x]
npm run typecheck - [x]
npm test— new suiteapps/sheets/tests/lazy-find.test.ts(20 tests) passes, and the rest of the sheets suite passes except tests that require the Rust sidecar binary / long perf timeouts, which cannot run on this Windows machine (no MSVC toolchain for the cargo build; see below). Those code paths are untouched by this PR and are exercised by CI's Ubuntu job, which builds the sidecar before running vitest.
List any checks not run and explain why:
- Sidecar-dependent tests (
xlsx-sidecar,xlsx-recalc,xlsx-streaming-save, onexlsx-borderscase) fail locally withENOENT ...xlsx-sidecar.exebecause building the Rust sidecar here requires MSVC Build Tools that are not installed;cargo buildfails at the linker step. No Rust code is modified by this PR, and CI builds the sidecar before running the suite. -
npm run licensespasses (the two newly declared deps were already in the tree and allowlisted).
Manual verification path for reviewers without a large fixture handy: open any .xlsx big enough to stream (> ~10k cells works), do not scroll, press Ctrl+F, and search for text that exists far below row 0 — before this PR it reports no matches; after, the count appears and Enter jumps to and selects the real cell.
Not applicable — no visible chrome changes; the difference is the Find dialog's match count/jump behavior on large streamed workbooks.
- [x] The change is focused and does not include unrelated reformatting or refactoring.
- [x] User-facing strings use the existing i18n resources (
appFindScanTruncatedadded to all 19 locale blocks instrings-app.ts). - [x] File open/save changes include an appropriate round-trip or fidelity test. — N/A: nothing in the open/save paths is touched; replacements go through the existing edit journal that save already replays.
Resolution of the PowerShell Installation Bug
I addressed a critical failure in the installation script that caused it to crash on fresh Windows systems whenever the user had PowerShell StrictMode enabled. The issue stemmed from a variable, specifically $script: LastResolver, that was only initialized within a specialized branch of the logic, leaving it undefined for most users and triggering a fatal exception during the reporting phase. By ensuring this variable is initialized to a default value before the report is generated, I successfully prevented the script from attempting to access a null reference.
In addition to this primary fix, I collaborated on refining the diagnostic output to improve how the system handles path resolution failures a contribution that ensures the final JSON report clearly distinguishes between a skipped process and one that never executed. By integrating this logic, I provided the clarity needed to debug future installation issues—effectively closing the loop on the reported bug—and verified the solution through a series of contract tests confirming the script now handles StrictMode environments without interruption.
install.ps1 no longer crashes at line 367 under StrictMode (#93017, salvage #93020)
#93398
install.ps1 no longer crashes at line 367 on fresh Windows installs when the caller's PowerShell session has StrictMode enabled. $script:LastResolver was only assigned on the rare 8.3-short-path branch; every ordinary machine reached the resolved-path report with the variable unset → fatal InvalidOperation before any install stage ran.
Salvage of #93020 by @liuhao1024 (first-filed, primary credit; authorship preserved) + the 'skipped-long-path' diagnostics hunk from #93100 (Co-authored-by: @aniruddhaadak80). Closes #93017.
-
scripts/install.ps1: initialize$script:LastResolver = 'none'before the report; early long-path return now records'skipped-long-path'so the report distinguishes "skipped" from "never ran" -
tests/test_install_ps1_resolver_strictmode.py: 3 source-contract tests (init exists, init precedes resolver-run and report-read, early-return pinned)
| Before (main) | After (branch) | |
|---|---|---|
pwsh 7.4.6 StrictMode, -ShowResolvedPaths
|
InvalidOperation at install.ps1:367, exit 1 | clean JSON report, "resolver":"skipped-long-path", exit 0 |
| contract tests vs base install.ps1 | 2 failed, 1 passed | 3 passed |
Live repro on real PowerShell (portable 7.4.6), not simulated.
Fixing Order-Dependent Test Failures in Agent Prompts
I recently resolved a persistent issue where running test files together caused failures due to shared state pollution. By implementing a robust cleanup process for the truncation warning context variable and updating the agent stub, I ensured that tests run reliably regardless of their execution order.
prompt_builder/system_prompt tests no longer fail when run together (#93018, salvage #93047)
#93395
tests/agent/test_prompt_builder.py and tests/agent/test_system_prompt.py now pass together in either order. The former leaked truncation warnings into a shared ContextVar; the latter's agent stub lacked _emit_status, so the leaked drain crashed with AttributeError — a phantom red for anyone running both files.
Salvage of #93047 by @aniruddhaadak80 (authorship preserved; BOM stripped from commit message) + our fixup making the drain fire before AND after each test. Closes #93018.
-
tests/agent/test_prompt_builder.py: autouse fixture draining the truncation-warning ContextVar (before + after) -
tests/agent/test_system_prompt.py:_make_agent()stub gains a no-op_emit_status
| Before (main) | After (branch) | |
|---|---|---|
| both files, order A | 1 failed (AttributeError at system_prompt.py:927), 104 passed | 105 passed |
| both files, order B | — | 105 passed |
Test-only change.
Fixing the Persistent Compression Lock
I am happy to share that I have finally resolved the issue where short sessions would permanently disable auto-compaction. Previously, the system incorrectly counted structural no-ops as ineffective compression strikes. Because two of these strikes were enough to latch the breaker for the entire life of a session, compaction would stop running even after a session grew significantly. This caused context to balloon and made every turn increasingly expensive.
To fix this, I updated the logic in agent/context_compressor.py so that structural no-ops now trigger a transient in-memory backoff for 300 seconds instead of applying durable strikes. This backoff is cleared whenever a manual compression command is used, a compaction cycle completes, or a session is reset. I also ensured that genuine attempted but underperformed compression verdicts still count as strikes to maintain our anti-thrash protections. I have aligned all relevant test files to this new contract and added a new test suite to ensure this behavior remains stable. This change effectively prevents the system from getting stuck while still protecting us from unnecessary overhead.
Short sessions no longer permanently disable auto-compaction (#93022, salvage #93093)
#93394
Auto-compaction no longer disables itself permanently on sessions that start too short to compress. The anti-thrash breaker counted structural no-ops (insufficient_messages, no_compressible_window, empty_post_handoff_window) as ineffective-compression strikes; two such no-ops latched the ≥2 breaker for the life of the session, so compaction never ran even after the session grew — context ballooned and every turn got more expensive.
Salvage of #93093 by @aniruddhaadak80 (authorship preserved). Closes #93022.
-
agent/context_compressor.py: structural no-ops arm a transient in-memory 300s backoff (structural_backoff:<s>block reason) instead of durable strikes; cleared on/compress(force), completed compaction, and session reset. Genuine attempted-but-underperformed verdicts still strike. - Tests: new
test_context_compressor_structural_backoff.py+ 4 existing anti-thrash test files aligned to the new contract
| Before (main) | After (branch) | |
|---|---|---|
| 2 compress calls on 5-msg session | strikes 1→2, breaker latched (ineffective) |
strikes stay 0, backoff armed |
| Session grown to 45 msgs w/ compressible material | still blocked forever | eligible again after backoff elapses |
/compress (force) |
n/a | bypasses backoff (verified) |
| PR test files (5) | — | 40 passed |
Preserves #40803's anti-refire guarantee (same 300s cadence as the existing recovery probe). Note: overlaps open #88388 (insufficient_messages prune-first) — that PR will need a small rebase of one hunk if salvaged later.
Cleanup of Legacy Cloud Port Allocation
I have successfully completed the cleanup of the RealLaunchEngine._allocate_port method, which had become obsolete following the architectural improvements made in #5189. By removing this helper and its associated state, we have eliminated a workaround that was originally implemented to force local and remote port alignment—a constraint that no longer exists in our current infrastructure.
I have updated the call sites to pass the registry default directly, ensuring that provision and register processes remain synchronized. This change simplifies our codebase by removing unnecessary logic and documentation notes, while also ensuring that newly provisioned EC2 gateways now utilize the standard default port as intended. We verified these changes across our cloud launch test suites to ensure that this shift in provisioning behavior maintains the stability and reliability of our deployment pipeline.
refactor(cloud): let a provisioned crew take the stock port
#5351
RealLaunchEngine._allocate_port exists only to work around a constraint that no longer exists. It was written because the instance tunnel forced local_port == remote_port and hard-failed when that port was busy, so a cloud crew registered on the default dashboard port could never be connected. #5189 removed that constraint — the hub now allocates its local forward port independently — and the helper's own docstring conceded it "could simply pass the registry default". The deletion was asked for three times in #5189's First Principles review and deferred there only on blast-radius grounds (fixes #5253).
Leaving it costs every future launch a fresh, pointless port allocation on both ends of one crew, keeps an instances-registry read on the cloud provisioning path for information the registry no longer needs, and forces every reader to reconstruct the dead local==remote rule to understand two lines of kwargs. The hedge "still mildly useful" is not a requirement; this makes the removal owned.
With the tunnel's local port independent, a crew no longer needs a unique remote port: EC2 hosts are isolated per crew, so sharing the stock remote port cannot collide across crews, and within one host there is exactly one gateway. Both ends therefore take their own defaults, which already agree:
-
provision()callsec2.deploy(...)with nodashboard_portoverride → the CloudFormation stack binds itsDashboardPortdefault (5476, percloud/templates/kirocrew-ec2.yaml). -
register()callsregister_instance(...)with noremote_port→ its signature defaultDEFAULT_REMOTE_DASHBOARD_PORT = 5476(cloud/connect.py), the same number. -
_allocate_port, the memoisedself._port, and the now-unusedPortAllocator/InstancesRegistryimports are deleted.
Alternatives: keeping the helper "just in case" was the exact state being corrected; making the default explicit by passing constants at both call sites would duplicate two spellings of one fact instead of deleting them.
TestRealEngineGatewayPort in test/test_cloud_launch_job.py is rewritten to pin the new contract: after provision + register, ec2.deploy received no dashboard_port key and register_instance received no remote_port key — so any reintroduced allocation step turns the suite red. The three tests that existed to pin the old allocation behaviour (same port both ends, skip registry ports, survive an unreadable registry) are deleted along with the behaviour they described.
Full local run of the touched surface, Windows 11 / Python 3.12:
pytest test/test_cloud_launch_job.py test/test_cloud_cli.py test/test_cloud_wizard.py \
test/test_cloud_login.py test/test_cloud_connect.py
→ 142 passed, 3 skipped
Plus flake8, isort, mypy clean on the changed module. Both changed files sit in .github/black-baseline.txt; their pre-existing formatting state is untouched and the diff adds no new black findings.
Fixes #5253
- [x] Single commit with a Conventional Commits title (
refactor: ...) - [x] Existing tests pass and new tests added for new functionality
- [x] Self-review completed; code follows project style guidelines
- [x] Documentation updated (if applicable) — N/A: no documented behaviour changes; the helper's docstring left with it
- [x] No secrets, credentials, or internal references in the diff
What the day taught me
Three things stuck with me. First, a tiny reproduction is worth an hour of staring. Every one of these bugs became obvious the moment I could trigger it on demand. Second, tests that fail first are the cheapest proof that a fix does something. They also protect the next person who touches the code. Third, reading the surrounding code before writing anything saves more time than it costs. Every repo already had conventions for tests, changesets, and commit style, and following them made review easy for everyone.
Wrap up
Three projects, three fixes, each shipped with failing first tests and green CI. The challenge closes on August 24, so if you have been waiting for a reason to send your first bug fix, this is a good one. Pick an issue that annoys you, shrink it until it fits on one screen, and make the test prove you killed it.
Top comments (0)