Close #1014, ALKiP notifications by plocket · Pull Request #1041 · SuffolkLITLab/ALKiln

plocket · 2026-02-02T14:20:46Z

In this PR, I have:

Added tests for any new features or bug fixes (if relevant)
Added my changes to the CHANGELOG.md at the top, under the "Unreleased" section
Ensured issues that this PR closes will be automatically closed

Reason for this PR

[Early PR for quick top-level sanity check review, failing tests are fine for now.]
Summary: Add notifications to ALKilnInThePlayground (ALKiP) for ALKiln changes
Details:
This is a testing framework. One of the environments in which it runs is on a test author's own server. ALKiP is a package of mine that helps authors runs these tests on their server. I work hard to ensure backwards compatibility, but it can be useful to keep both up to date with the other and sometimes to notify authors of other changes to their server that might affect how ALKiP is running. It's hard for authors to tell which versions match up with each other and when to update in general.

This ALKiln PR generates HTML that ALKiP can use to add notifications about relevant changes to ALKiln, to the author's server, or about whatever else ALKiln thinks is helpful. We will use notifications sparingly.

Demo of the visual appearance of a notification. Visual preview:

Links to any solved or related issues

Closes #1014

Any manual testing I have done to ensure my PR is working

Publishing to npm and testing on ALKiP

any announcements new ALKiln versions have even if ALKilnInThePlayground isn't itself updated. Closes #1014 Co-authored-by: will-morningstar <mxwillmorningstar@gmail.com>

…n into 1014_breaking_news_2

tobynorth

Looks good to me! Didn't find any major issues. One high-level, almost certainly out of scope question for my own understanding: it seems like the need for this PR arose out of ALKiln and ALKiP being separate packages -- why is that? (would you ever want to install ALKiP without wanting to install ALKiln?)

And a general question: how are all the logging changes (artifacts.js, validate.js, Log.js) and path changes (files.js, session_vars.js, set_sources_path.js, scope.js) related to the new notifications feature? For clarity, I'd suggest either putting something about these changes in the PR description or moving them to separate PR(s).

tobynorth · 2026-02-03T21:35:03Z

lib/setup/artifacts.js

 const argv = require(`minimist`)( process.argv.slice(2) );
-const artifacts_path = files.make_artifacts_folder( argv.path );
+
+const artifacts_path = files.make_artifacts_folder();


nit: argv is no longer used -- consider either cleaning it up or (if this was accidental) restoring it as an arg to make_artifacts_folder

tobynorth · 2026-02-05T22:12:05Z

lib/steps.js

+  // Save Scenario data in Scenario folder
+  // '@alkiln @generated'
+  // console.log( JSON.stringify( scenario ) );


question/nit: did you mean to commit these comments?

tobynorth · 2026-02-05T22:16:04Z

package.json

 {
  "name": "@suffolklitlab/alkiln",
-  "version": "5.15.4",
+  "version": "5.15.0-feat-news-17",


question (non-blocking): just to satisfy my own curiosity, what's going on with the version number here? (in particular, why did the patch version number drop from 4 to 0?)

tobynorth · 2026-02-05T22:56:44Z

lib/steps.js

  scope.paths.scenario_report = `${ scope.paths.scenario }/report.txt`;
+  // If it's a generated Scenario, save the `.feature` file in here
+  // GENERATED_FILES_FOLDER_NAME
+  if ( scenario.gherkinDocument.uri.includes("_alkiln_generated") ) {


nit: _alkiln_generated is a magic string, ideally would be defined in a more maintainable location. (Where did it come from, anyway? I don't see any other references to it in this PR)

tobynorth · 2026-02-05T23:02:46Z

lib/breaking_news.js

+   * Note that the function prints the value to the console before returning the
+   *     value. This is the only way ALKilnInThePlayground can get the value.


question: why is this? Seems cleaner to use a temp file; is that not feasible for some reason?

tobynorth · 2026-02-06T18:09:14Z

lib/breaking_news.js

+    for ( let subhead of shuffled_subheads ) {
+
+      if ( subhead.includes(`!`) && used_exclamation ) { continue; }
+      if ( subhead.includes(`?`) && used_question ) { continue; }
+      if ( subhead.includes(`"`) && used_quotes ) { continue; }
+      if ( subheads_used[ subhead ]) { continue; }
+
+      if ( subhead.includes(`!`)) { used_exclamation = true; }
+      if ( subhead.includes(`?`)) { used_question = true; }
+      if ( subhead.includes(`"`)) { used_quotes = true; }
+
+      final_subhead = subhead;
+      subheads_used[ subhead ] = true;
+    }


suggestion: it feels like it would be a little cleaner to refactor this into get_shuffled_subheads by just ensuring that a max of 1 subhead with each checked char (and no duplicate subheads) are added to the return array. Then in this function, you could just pop or shift an element off the array to pass to get_one_articles_parts

tobynorth · 2026-02-06T18:20:22Z

lib/breaking_news.js

+  let html_paragraphs = [];
+  for ( let one_paragraph of paragraphs ) {
+    html_paragraphs.push(`<p class="body_copy">${ one_paragraph }</p>`);
+  }
+  return html_paragraphs;


suggestion: this could be simplified to:

Suggested change

let html_paragraphs = [];

for ( let one_paragraph of paragraphs ) {

html_paragraphs.push(`<p class="body_copy">${ one_paragraph }</p>`);

}

return html_paragraphs;

return paragraphs.map(paragraph => `<p class="body_copy">${ paragraph }</p>`);

tobynorth · 2026-02-06T18:33:48Z

lib/breaking_news.js

+  if ( typeof list_or_str === `string` ) {
+    indent_str = `  `.repeat( indent );
+    return [ indent_str + list_or_str ];
+  }
+
+  let flattened = [];
+  for ( let nested_str_or_list of list_or_str ) {
+    let result = indent_and_flatten_html_list(
+      nested_str_or_list,
+      indent + 1
+    );
+    // // .concat() feels evil because it accepts a string as well
+    // flattened = flattened.concat( result );
+    flattened.push( ...result );
+  }
+
+  return flattened;


suggestion: I think this could be simplified to:

Suggested change

if ( typeof list_or_str === `string` ) {

indent_str = ` `.repeat( indent );

return [ indent_str + list_or_str ];

}

let flattened = [];

for ( let nested_str_or_list of list_or_str ) {

let result = indent_and_flatten_html_list(

nested_str_or_list,

indent + 1

);

// // .concat() feels evil because it accepts a string as well

// flattened = flattened.concat( result );

flattened.push( ...result );

}

return flattened;

return list_or_str.flatMap(nested_str_or_list =>

Array.isArray(nested_str_or_list)

? indent_and_flatten_html_list(nested_str_or_list, indent + 1)

: ` `.repeat(indent) + nested_str_or_list

);

tobynorth · 2026-02-06T18:41:36Z

lib/breaking_news.js

+}
+
+
+function get_semver_parts( version_str ) {


suggestion: Consider using a 3rd party library to replace all your semver-related functions if you haven't already (example). Since this is neither related to the core ALKiln logic nor (it seems) something ALKiln needs to go outside the semver spec for, probably better to let someone else maintain it

tobynorth · 2026-02-06T18:46:49Z

lib/breaking_news.js

+    // If we switch to module (import) instead of common (require), use
+    // https://nodejs.org/api/esm.html#importmetafilename for file path
+    let most_recent_file_sha = execSync(
+      `git rev-list -1 HEAD -- "${ __filename }"`


question: this is running on users' servers, right? Is it safe to assume they have git installed and have cloned the ALKiln repo locally?...

plocket and others added 16 commits June 14, 2025 22:21

Add "Breaking news" feature so ALKilnInThePlayground can show

c49cb80

any announcements new ALKiln versions have even if ALKilnInThePlayground isn't itself updated. Closes #1014 Co-authored-by: will-morningstar <mxwillmorningstar@gmail.com>

Debug ALKiP news

6346c16

Add logs to help with ALKiP development

413aad8

Remove erroring code so we can see what the logs say

0589d1f

Use cwd when creating the path to make files

5e3946a

Use cwd liberally to get a dynamic path to root (for ALKiP)

1050612

Debug node can't find feature files after already finding them ourselves

d457454

Use __dirname in readFileSync to (supposedly) get correct relative path

5c72d3b

Remove *.feature (filename glob) from what is supposed to be a path glob

7cb08ff

Update paths to pass local tests, tweak some names

eae2510

Update paths to pass local tests, tweak some names

6b362a5

Generate html that shows ALKiln notifications ("breaking news")

ff22c9c

Update paths to pass local tests, tweak some names

d56625d

Generate html that shows ALKiln notifications ("breaking news")

9e2f223

Merge branch '1014_breaking_news_2' of github.com:SuffolkLITLab/ALKil…

a506590

…n into 1014_breaking_news_2

Bump experimental version after merging v5

8f34462

plocket requested a review from BryceStevenWilley as a code owner February 2, 2026 14:20

Merge branch 'v5' and bump experimental version

067d26b

plocket marked this pull request as draft February 2, 2026 14:27

tobynorth approved these changes Feb 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Close #1014, ALKiP notifications#1041

Close #1014, ALKiP notifications#1041
plocket wants to merge 17 commits intov5from
1014_breaking_news_2

plocket commented Feb 2, 2026 •

edited

Loading

Uh oh!

tobynorth left a comment

Uh oh!

tobynorth Feb 3, 2026

Uh oh!

tobynorth Feb 5, 2026

Uh oh!

tobynorth Feb 5, 2026

Uh oh!

tobynorth Feb 5, 2026

Uh oh!

tobynorth Feb 5, 2026

Uh oh!

tobynorth Feb 6, 2026

Uh oh!

tobynorth Feb 6, 2026

Uh oh!

tobynorth Feb 6, 2026

Uh oh!

tobynorth Feb 6, 2026

Uh oh!

tobynorth Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		* Note that the function prints the value to the console before returning the
		* value. This is the only way ALKilnInThePlayground can get the value.

Conversation

plocket commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reason for this PR

Links to any solved or related issues

Any manual testing I have done to ensure my PR is working

Uh oh!

tobynorth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

plocket commented Feb 2, 2026 •

edited

Loading