• Joined on 2026-08-13
hermes commented on pull request dc/dc#306 2026-08-17 14:59:10 +00:00
fix: core major bump plus autofix of viya context on deploy

Hermes Agent Code Review

hermes commented on pull request dc/dc#306 2026-08-17 14:59:10 +00:00
fix: core major bump plus autofix of viya context on deploy

Suggestion: the new DC.html context-rewrite logic (lines 214-267) has no test. testsetup.sas runs makedata but doesn't assert that contextname="..." in DC.html was updated to &_contextname. Consider reading DC.html back after makedata and asserting the contextname= value equals &defaultcontext.

hermes commented on pull request dc/dc#306 2026-08-17 14:59:09 +00:00
fix: core major bump plus autofix of viya context on deploy

Suggestion: with truncover, any DC.html line exceeding lrecl=32767 would be silently truncated and then rewritten, corrupting the file. DC.html is the small SASjs loader shell (assets are split into separate files, and Angular has inlineCritical: false), so 32767 is ample in practice. A defensive check on &syserr / max line length after this step would make a silent-truncation regression observable.

hermes commented on pull request dc/dc#306 2026-08-17 14:59:09 +00:00
fix: core major bump plus autofix of viya context on deploy

Suggestion: the same fileref &dchtml_fref is reassigned here for write mode after being used for read on line 223. A one-line comment noting the reassignment for write would help readers — reusing one fileref for both directions is non-obvious.

hermes commented on pull request dc/dc#306 2026-08-17 14:59:09 +00:00
fix: core major bump plus autofix of viya context on deploy

Warning: symget('_contextname') is concatenated directly into both the regex pattern and the replacement string. If a compute context name ever contains regex metacharacters (`

hermes commented on pull request dc/dc#306 2026-08-17 14:59:09 +00:00
fix: core major bump plus autofix of viya context on deploy

Warning: _contextname is an auto/URL macro variable and is already global — this %global is redundant. Consider moving it up next to %global dcpath ADMIN; (line 27) for consistency.

hermes commented on pull request dc/dc#294 2026-08-17 12:53:09 +00:00
Version 7.13 Release

Test Coverage Report

SASjs compile/coverage run via npx @sasjs/cli c -t server-ci (local compile only — no server required).

hermes commented on pull request dc/dc#294 2026-08-17 12:52:56 +00:00
Version 7.13 Release

Hermes Agent Code Review

hermes commented on pull request dc/dc#294 2026-08-17 12:52:56 +00:00
Version 7.13 Release

Performance note (non-blocking): the dataSourceUnchanged.find(...) inside the dataSource.forEach + for (const formulaRule ...) loop is O(rows × formulaRules × unchangedRows). The same PK-matching find-inside-forEach pattern appears in findOverwrittenCells.ts and classifyRow.ts. Fine for typical editor row counts; if very large staging tables become a supported use case, a one-time Map keyed by the joined PK values would drop this to O(rows × formulaRules).

hermes commented on pull request dc/dc#294 2026-08-17 12:52:56 +00:00
Version 7.13 Release

Low-priority edge case: the quotedSpanPattern regex `"[^"]*"

hermes commented on pull request dc/dc#294 2026-08-17 12:52:52 +00:00
Version 7.13 Release

Hermes Agent Code Review

hermes commented on pull request dc/dc#294 2026-08-17 12:52:52 +00:00
Version 7.13 Release

Performance note (non-blocking): the dataSourceUnchanged.find(...) inside the dataSource.forEach + for (const formulaRule ...) loop is O(rows × formulaRules × unchangedRows). The same PK-matching find-inside-forEach pattern appears in findOverwrittenCells.ts and classifyRow.ts. Fine for typical editor row counts; if very large staging tables become a supported use case, a one-time Map keyed by the joined PK values would drop this to O(rows × formulaRules).

hermes commented on pull request dc/dc#294 2026-08-17 12:52:52 +00:00
Version 7.13 Release

Low-priority edge case: the quotedSpanPattern regex `"[^"]*"

hermes commented on pull request dc/dc#294 2026-08-17 12:51:57 +00:00
Version 7.13 Release

test

hermes commented on pull request dc/dc#305 2026-08-17 10:57:48 +00:00
fix: address hermes review feedback (formula quoting, cell revert, addRow guard)

Hermes Agent Code Review

hermes commented on pull request dc/dc#305 2026-08-17 10:57:48 +00:00
fix: address hermes review feedback (formula quoting, cell revert, addRow guard)

The if (this.dataSource[newIndex]) guard correctly skips noLinkOption, and seedFormulaValuesForRow/updateEditStatusForRow both have their own missing-row early returns — consistent. Note that hot.selectCell(newIndex, 0) and hot.render() below still run unconditionally when dataSource[newIndex] is undefined. If the async-binding scenario this guards against ever occurs, consider widening the guard (or an early return) to cover those calls too. Not blocking — the guard as-is is strictly better than before.

hermes commented on pull request dc/dc#305 2026-08-17 10:57:48 +00:00
fix: address hermes review feedback (formula quoting, cell revert, addRow guard)

Warning: Number('') and Number(' ') return 0, not NaN, so this guard does not fire for an empty/whitespace original value on a numeric column — resolveRevertedCellValue('', true) returns 0 instead of '', writing 0 into an originally-blank numeric cell. SAS special missings (., .A.Z, .S) all correctly fall back to text. If blanks can occur, consider `const trimmed = rawValueText.trim(); ... trimmed === ''

hermes commented on pull request dc/dc#305 2026-08-17 10:57:48 +00:00
fix: address hermes review feedback (formula quoting, cell revert, addRow guard)

Good doc comment — documenting a fragile cross-repo structural assumption (the HTTP-key "same text in both fields" invariant) is the right call. No code change, no test gap created.

hermes commented on pull request dc/dc#305 2026-08-17 10:57:48 +00:00
fix: address hermes review feedback (formula quoting, cell revert, addRow guard)

Correct: doubling embedded " is the right string-literal escape convention for HyperFormula. Both new spec cases (john"doe, a "quoted" value) assert it end-to-end. String(value ?? '') also correctly normalizes undefined to empty before escaping. Looks good.

hermes commented on pull request dc/dc#294 2026-08-14 11:22:29 +00:00
Version 7.13 Release

Hermes Agent Code Review